Hi, On Thu, Jul 9, 2020 at 4:05 PM Abhishek Bhardwaj <abhishekbh@xxxxxxxxxx> wrote: > > > > On Thu, Jul 9, 2020 at 3:43 PM mark gross <mgross@xxxxxxxxxxxxxxx> wrote: >> >> On Thu, Jul 09, 2020 at 12:42:57PM -0700, Doug Anderson wrote: >> > Hi, >> > >> > On Thu, Jul 9, 2020 at 3:51 AM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote: >> > > >> > > Abhishek Bhardwaj <abhishekbh@xxxxxxxxxx> writes: >> > > > This change adds a new kernel configuration that sets the l1d cache >> > > > flush setting at compile time rather than at run time. >> > > > >> > > > The reasons for this change are as follows - >> > > > >> > > > - Kernel command line arguments are getting unwieldy. These parameters >> > > > are not a scalable way to set the kernel config. They're intended as a >> > > > super limited way for the bootloader to pass info to the kernel and >> > > > also as a way for end users who are not compiling the kernel themselves >> > > > to tweak the kernel behavior. >> > > > >> > > > - Also, if a user wants this setting from the start. It's a definite >> > > > smell that it deserves to be a compile time thing rather than adding >> > > > extra code plus whatever miniscule time at runtime to pass an >> > > > extra argument. >> > > > >> > > > - Finally, it doesn't preclude the runtime / kernel command line way. >> > > > Users are free to use those as well. >> > > >> > > TBH, I don't see why this is a good idea. >> > > >> > > 1) I'm not following your argumentation that the command line option is >> > > a poor Kconfig replacement. The L1TF mode is a boot time (module >> > > load time) decision and the command line parameter is there to >> > > override the carefully chosen and sensible default behaviour. >> > >> > When you say that the default behavior is carefully chosen and >> > sensible, are you saying that (in your opinion) there would never be a >> > good reason for someone distributing a kernel to others to change the >> > default? Certainly I agree that having the kernel command line >> > parameter is nice to allow someone to override whatever the person >> > building the kernel chose, but IMO it's not a good way to change the >> > default built-in to the kernel. >> > >> > The current plan (as I understand it) is that we'd like to ship >> > Chromebook kernels with this option changed from the default that's >> > there now. In your opinion, is that a sane thing to do? >> > >> > >> > > 2) You can add the desired mode to the compiled in (partial) kernel >> > > command line today. >> > >> > This might be easier on x86 than it is on ARM. ARM (and ARM64) >> > kernels only have two modes: kernel provides cmdline and bootloader >> > provides cmdline. There are out-of-mainline ANDROID patches to >> > address this but nothing in mainline. >> > >> > The patch we're discussing now is x86-only so it's not such a huge >> > deal, but the fact that combining the kernel and bootloader >> > commandline never landed in mainline for arm/arm64 means that this >> > isn't a super common/expected thing to do. >> > >> > >> > > 3) Boot loaders are well capable of handling large kernel command lines >> > > and the extra time spend for reading the parameter does not matter >> > > at all. >> > >> > Long command lines can still be a bit of a chore for humans to deal >> > with. Many times I've needed to look at "/proc/cmdline" and make >> > sense of it. The longer the command line is and the more cruft >> > stuffed into it the more of a chore it is. Yes, this is just one >> > thing to put in the command line, but if 10 different drivers all have >> > their "one thing" to put there it gets really long. If 100 different >> > drivers all want their one config option there it gets really really >> > long. IMO the command line should be a last resort place to put >> > things and should just contain: >> >> This takes me back to my years doing android kernel work for Intel, I'm glad >> those are over. Yes, the android kernel command lines got hideous, I think we >> even had patches to make the cmdline buffer bigger than the default was. >> >> From a practical point of view the command line was part of the boot image and >> cryptography protected so it was a handy way to securely communicate parameters >> from the platform to the kernel, drivers and even just user mode. It got >> pretty ugly but, it worked (mostly). >> >> What I don't get is why pick on l1tf in isolation? > > Because this is what we needed given the state of our projects. Right. Basically the flow is: 1. Someone comes up with a tweak that they think some people might want to try out but it's not expected to be the default for anyone: add a module parameter. 2. Someone comes around and says: I think it's sensible to change the default for a whole class of devices, but another class of devices can still use the old default: add a config option in addition to the kernel parameter. If #2 never happens then it can stay a module parameter forever. If #2 happens that's fine--it doesn't really hurt to add a config option for it. >> There are a bunch of >> command line parameters similar to l1tf. Would a more general option make >> sense? > > Maybe. Given how much resistance this CONFIG has met, I can only imagine the resistance > upon introducing a more reaching configuration. I also think there's a fine line between YAGNI (You Ain't > Gonna Need It) vs planning for the future. I don't want to introduce a big CONFIG that won't be used for > anything else. I'd rather we cross that bridge when someone else feels the same way about other options. What are you envisioning? I guess the "generic" solution is to add cmdline extension (being able to combine kernel and bootloader cmdline) to all platforms in a somewhat uniform way without breaking any backward compatibility. You'd still end up with a really ugly "/proc/cmdline" if you had too many options there, but I guess it wouldn't be the end of the world. Getting this done on all platforms doesn't seem like it'd be an easy task, though. >> Anyway, I think there is a higher level issue you are poking at that might be >> better addressed by talking about it directly. > > > I am new to this process. I don't know who to "convince" or talk about these issues. Who is the final authority on > this ?. I don't think appeasing every concern is going to be productive for anyone. I've offered to change the implementation, > if that is what's required. However, if the final authority is against a CONFIG, I don't really know how to proceed. > >> >> >> --mark >> >> > > > -- > Abhishek
