Re: [PATCH v7 08/12] xen: add /buildinfo/config entry to hypervisor filesystem

[Jürgen Groß <jgross@xxxxxxxx>]

On 28.04.20 13:23, George Dunlap wrote:
> > On Apr 28, 2020, at 9:39 AM, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >
> > On 28.04.2020 10:24, George Dunlap wrote:
> > > > On Apr 28, 2020, at 8:20 AM, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> > > > On 27.04.2020 18:25, George Dunlap wrote:
> > > > > If Jan is OK with it simply being outside CONFIG_EXPERT, then great.  But if he 
> > > > > insists on some kind of testing for it to be outside of CONFIG_EXPERT, then 
> > > > > again, the people who want it to be security supported should be the ones who 
> > > > > do the work to make it happen.
> > > > I don't understand this part, I'm afraid: Without a config option,
> > > > the code is going to be security supported as long as it doesn't
> > > > get marked otherwise (experimental or what not). With an option
> > > > depending on EXPERT, what would become security unsupported is the
> > > > non-default (i.e. disabled) setting. There's not a whole lot to
> > > > test there, it's merely a formal consequence of our general rules.
> > > > (Of course, over time dependencies of other code may develop on
> > > > the information being available e.g. to Dom0 userland. Just like
> > > > there's Linux userland code assuming the kernel config is
> > > > available in certain ways [I don't necessarily mean the equivalent
> > > > of hypfs here], to then use it in what I'd call abusive ways in at
> > > > least some cases.)
> > > Here’s an argument you might make:
> > >
> > > “As a member of the security team, I don’t want to be on the hook for issuing XSAs for code 
> > > which isn’t at least smoke-tested.  Therefore, I oppose any patch adding CONFIG_HYPFS outside of 
> > > CONFIG_EXPERT, *unless* there is a concrete plan for getting regular testing for CONFIG_HYPFS=n.”
> > >
> > > I’m not saying that’s an argument you *should* make.  But personally I don’t 
> > > have a strong argument against such an argument. So, it seems to me, if you did make it, you have a 
> > > reasonable chance of carrying your point.
> > >
> > > Now consider this hypothetical universe where you made that argument and nobody opposed it.  
> > > In order to get a particular feature (CONFIG_HYPFS=n security supported), there is extra work 
> > > that needs to be done (getting CONFIG_HYPFS=n tested regularly).  My point was, the 
> > > expectation should be that the extra work will be done by the people who want or benefit from 
> > > the feature; the series shouldn’t be blocked until Juergen implements CONFIG_HYPFS=n 
> > > testing (since he doesn’t personally have a stake in that feature).
> > >
> > > Now obviously, doing work to help someone else out in the community is of 
> > > course a good thing to do; it builds goodwill, uses our aggregate resources 
> > > more efficiently, and makes our community more enjoyable to work with. But the 
> > > goodwill primarily comes from the fact that it was done as a voluntary choice, 
> > > not as a requirement.
> > >
> > > Juergen was balking at having to do what he saw as extra work to implement CONFIG_HYPFS.  I 
> > > wanted to make it clear that even though I see value in having CONFIG_HYPFS, *he* 
> > > doesn’t have to do the work if he doesn’t want to (although it would certainly 
> > > be appreciated if he did).  And this paragraph was extending the same principle into the 
> > > hypothetical universe where someone insisted that CONFIG_HYPFS=n had to be tested before 
> > > being security supported.
> > >
> > > Hope that makes sense. :-)
> > Yes, it does, thanks for the clarification. I can see what you describe
> > as a valid perspective to take, but really in my request to Jürgen I
> > took another: Now that we have Kconfig, additions of larger bodies of
> > code (possibly also just in terms of binary size) should imo generally
> > be questioned whether they want/need to be built for everyone. I.e. it
> > is not to be left to people being worried about binary sizes to arrange
> > for things to not be built, but for people contributing new but not
> > entirely essential code to consider making it option from the very
> > beginning.
> I think that’s a reasonable position to take, but needs to be balanced on the amount of work that 
> this would actually require.  If it only requires adding a handful of #ifdef’s and maybe making a 
> few stubs, then yes, asking the submitter to make the change makes sense.  But if it requires three dozen 
> #ifdef’s throughout the code and a fairly major architectural change, then I think it’s 
> reasonable for a submitter to push back.
>
> I don’t really understand why Juergen thinks adding CONFIG_HYPFS would cause a 
> lot of code churn; my argumentation here is based on the assumption that his 
> assessment is correct.
The main problem I'm seeing is the setting of runtime parameters.

This will need a complete different set of macros for defining those
parameters, split across multiple patches. And I'm fairly sure I'll
need to touch each custom_runtime_param handling function, too, in
order not to add dead code.


Juergen