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

Elliot Berman
2022-10-04T23:53:57Z

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