>>> On 29.08.18 at 14:36, <andrew.coop...@citrix.com> wrote: > On 21/08/18 11:44, Jan Beulich wrote: >> While commit 2a3b34ec47 ("x86/spec-ctrl: Yet more fixes for xpti= >> parsing") indeed fixed "xpti=dom0", it broke "xpti=no-dom0", in that >> this then became equivalent to "xpti=no". > > That was accidental, but the end result is consistent with other options. > > As with spec-ctrl, if someone wants to start making fine-grain control, > they should specify everything. There is a reason why the > > **WARNING: Any use of this option may interfere with heuristics. Use > with extreme care.** > > disclaimer exists.
As said ... >> In particular, the presence >> of "xpti=" alone on the command line means nothing as to which >> default is to be overridden; "xpti=no-dom0" ought to have no effect >> for DomU-s (and vice versa), as this is distinct from both >> "xpti=no-dom0,domu" and "xpti=no-dom0,no-domu". ... here, the current behavior is pretty counterintuitive. I'm also curious to know how this is "consistent with other options" - can you give two or three examples at least? >> Here as well as for "pv-l1tf=" I think there's no way around tracking >> the "use default" state separately for Dom0 and DomU-s. Introduce >> individual bits for this, and convert the variables' types (back) to >> uint8_t. > > The code below is getting unmanageably complicated. Given that its all > slowpath operations, I think switching to 4 separate int8_t's would be > better than trying to multiplex several tristates into the same byte. > It would also remove all of the constants. I can do that; it simply seemed more intrusive a change that way. >> Additionally the earlier change claimed to have got rid of the >> 'parameter "xpti" has invalid value "", rc=-22!' log message for "xpti" >> alone on the command line, which wasn't the case (the option took effect >> nevertheless). Fix this as well. > > The earlier change did do what the patch claimed, to the best of my > knowledge. Can you explain what is apparently still broken, because its > not clear from this description? The earlier commit claims the log message went away, which is not the case according to the testing that I had done with various option combinations while putting together this change. Hence this - else + else if ( *s ) rc = -EINVAL; break; change in both of the handlers >> Finally also support a "default" sub-option for "pv-l1tf=", just like >> "xpti=" does. > > No. Having "default" was a mistake for xpti= I would have objected to > if I'd noticed it. > > The default is not specifying the option in the first place. > "foo=default" is entirely redundant, It is not. I've said so a number of times before: You need a way to go back to the default when you want to override a stored portion of the command line that you can't edit while booting (as is minimally the case for xen.efi, which takes the command line out of a config file). >and worse, in combination with your > "Make "spec-ctrl=no" a global disable of all mitigations" patch: > > "spec-ctrl=0 $FOO=default" and > "$FOO=default spec-ctrl=0" > > now result in different things happening. And validly so: Order of options matters. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel