Re: [PATCH v4 10/14] gunyah: sysfs: Add node to describe supported features

From: Elliot Berman <quic_eberman@quicinc.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Bjorn Andersson <quic_bjorande@quicinc.com>,
	Murali Nalajala <quic_mnalajal@quicinc.com>,
	Trilok Soni <quic_tsoni@quicinc.com>,
	"Srivatsa Vaddagiri" <quic_svaddagi@quicinc.com>,
	Carl van Schaik <quic_cvanscha@quicinc.com>,
	Andy Gross <agross@kernel.org>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Jassi Brar <jassisinghbrar@gmail.com>,
	<linux-arm-kernel@lists.infradead.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Marc Zyngier <maz@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Jonathan Corbet <corbet@lwn.net>, "Will Deacon" <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	"Arnd Bergmann" <arnd@arndb.de>, <devicetree@vger.kernel.org>,
	<linux-doc@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 10/14] gunyah: sysfs: Add node to describe supported features
Date: Tue, 4 Oct 2022 16:53:33 -0700	[thread overview]
Message-ID: <7f161062-fe7a-05a0-2719-c92ab58f5ac6@quicinc.com> (raw)
In-Reply-To: <YzbbxuInZwILaflH@kroah.com>

On 9/30/2022 5:06 AM, Greg Kroah-Hartman wrote:
> On Wed, Sep 28, 2022 at 12:56:29PM -0700, Elliot Berman wrote:
>> Add a sysfs node to list the features that the Gunyah hypervisor and
>> Linux supports. For now, Linux support cspace (capability IDs) and
>> message queues, so only report those..
>>
>> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
>> ---
>>   Documentation/ABI/testing/sysfs-hypervisor-gunyah | 15 +++++++++++++++
>>   drivers/virt/gunyah/sysfs.c                       | 15 +++++++++++++++
>>   2 files changed, 30 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-hypervisor-gunyah b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
>> index 7d74e74e9edd..6d0cde30355a 100644
>> --- a/Documentation/ABI/testing/sysfs-hypervisor-gunyah
>> +++ b/Documentation/ABI/testing/sysfs-hypervisor-gunyah
>> @@ -1,3 +1,18 @@
>> +What:		/sys/hypervisor/gunyah/features
>> +Date:		October 2022
>> +KernelVersion:	6.1
>> +Contact:	linux-arm-msm@vger.kernel.org
>> +Description:	If running under Gunyah:
>> +		Space separated list of features supported by Linux and Gunyah:
>> +		"cspace": Gunyah devices
>> +		"doorbell": Sending/receiving virtual interrupts via Gunyah doorbells
>> +		"message-queue": Sending/receiving messages via Gunyah message queues
>> +		"vic": Interrupt lending
>> +		"vpm": Virtual platform management
>> +		"vcpu": Virtual CPU management
>> +		"memextent": Memory lending/management
>> +		"trace": Gunyah hypervisor tracing
> 
> Please no.  Why do you really need this type of list?  What hypervisor
> will NOT have them all present already?  Who will use this file and what
> will it be used for?
> 
> sysfs files should just be 1 value and not need to be parsed.  Yes, we
> have lists of features at times, but really, you need a very very good
> reason why this is the only way this information can be exposed and who
> is going to use it in order to be able to have this accepted.
> 

We're currently at phase where all the features implemented so far as 
considered part of the "base" featureset. We're thinking of future 
features implemented in Gunyah: userspace might need to know that some 
hypervisor feature is present and that it should make use of the feature 
instead of some fallback behavior.

I can drop this and it should be OK IMO to introduce it later if needed. 
The lack of the "gunyah/features" node would be sufficient for a 
userspace program to know that some new feature isn't present.

> thanks,
> 
> greg k-h