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

From: "Jürgen Groß" <jgross@suse.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien@xen.org>, Wei Liu <wl@xen.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Jan Beulich <jbeulich@suse.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v7 08/12] xen: add /buildinfo/config entry to hypervisor filesystem
Date: Mon, 27 Apr 2020 17:40:31 +0200	[thread overview]
Message-ID: <31c7f4fe-847c-96f5-e598-dba99b0bb61a@suse.com> (raw)
In-Reply-To: <51e0f0d2-f9ce-83fd-79fa-ae4805356612@suse.com>

Stefano,

On 06.04.20 14:29, Jan Beulich wrote:
> On 03.04.2020 17:45, Jürgen Groß wrote:
>> On 03.04.20 17:33, Jan Beulich wrote:
>>> On 03.04.2020 17:12, Jürgen Groß wrote:
>>>> On 03.04.20 16:31, Jan Beulich wrote:
>>>>> On 02.04.2020 17:46, Juergen Gross wrote:
>>>>>> --- a/xen/common/Kconfig
>>>>>> +++ b/xen/common/Kconfig
>>>>>> @@ -353,6 +353,16 @@ config DOM0_MEM
>>>>>>             Leave empty if you are not sure what to specify.
>>>>>>     +config HYPFS_CONFIG
>>>>>> +    bool "Provide hypervisor .config via hypfs entry"
>>>>>> +    default y
>>>>>
>>>>> My initial reaction was to ask for "depends on HYPFS", but then
>>>>> I noticed the earlier patch doesn't introduce such. Am I
>>>>> mis-remembering that it was agreed to make the whole thing
>>>>> possible to disable at least in EXPERT mode?
>>>>
>>>> No, I don't remember that agreement.
>>>>
>>>> And TBH I'm not sure this is a good idea, as that would at once make the
>>>> plan to replace at least some sysctl and/or domctl interfaces impossible
>>>> (like e.g. the last 3 patches of the series are doing already), or at
>>>> least would tie the related functionality to CONFIG_HYPFS.
>>>
>>> I think that would be fine - that's what config setting are for.
>>> Someone caring about space may not care about runtime setting of
>>> parameters.
>>
>> So right now it would start with a plain hypfs available or not.
>>
>> The next step would be in patch 12 to tell the user he will lose the
>> capability of setting runtime parameters.
>>
>> Another planned extension would be to control per-cpupool settings,
>> which would the go away (possibly functionality being unconditionally
>> available today).
>>
>> Next would be the lack of being able to control per-domain mitigations
>> like XPTI or L1TF, which I'd like to add.
>>
>> Another thing I wanted to add is some debugging stuff (e.g. to switch
>> lock profiling using hypfs).
>>
>> And the list will go on.
> 
> Understood.
> 
>> Does it really make sense to make a central control and information
>> interface conditional?
> 
> None of the above may be of interest to e.g. embedded use cases.
> 
>> I'd like at least a second opinion on that topic.
> 
> Yes, further opinions would surely help.

Any opinion on making hypfs configurable?

It would be quite some code churn and I want to avoid it in case you
see no benefit in configuring it out for embedded.

Juergen