Message-ID: <d0c9ac16-eb91-95cc-77bd-08e853e31bd5@linux.ibm.com>
Date:   Thu, 9 Dec 2021 16:59:21 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.3.2
Subject: Re: [PATCH v3] powerpc/pseries: read the lpar name from the firmware
Content-Language: en-US
To:     Nathan Lynch <nathanl@linux.ibm.com>
Cc:     linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
References: <20211203154321.13168-1-ldufour@linux.ibm.com>
 <87bl1so588.fsf@linux.ibm.com>
 <bbaa0d78-a09f-3ce3-25a9-67434039b741@linux.ibm.com>
 <878rwwny1l.fsf@linux.ibm.com>
 <21eb4749-42b1-da78-8833-00d360fa36e5@linux.ibm.com>
 <874k7jnmva.fsf@linux.ibm.com>
 <b6edbf96-4349-c39b-69ee-477b4fdef511@linux.ibm.com>
 <871r2lopes.fsf@linux.ibm.com>
From:   Laurent Dufour <ldufour@linux.ibm.com>
In-Reply-To: <871r2lopes.fsf@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 09/12/2021, 14:53:31, Nathan Lynch wrote:
> Laurent Dufour <ldufour@linux.ibm.com> writes:
>> On 08/12/2021, 16:21:29, Nathan Lynch wrote:
>>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>>>> On 07/12/2021, 18:07:50, Nathan Lynch wrote:
>>>>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>>>>>> On 07/12/2021, 15:32:39, Nathan Lynch wrote:
>>>>>>> Is there a reasonable fallback for VMs where this parameter doesn't
>>>>>>> exist? PowerVM partitions should always have it, but what do we want the
>>>>>>> behavior to be on other hypervisors?
>>>>>>
>>>>>> In that case, there is no value displayed in the /proc/powerpc/lparcfg and
>>>>>> the lparstat -i command will fall back to the device tree value. I can't
>>>>>> see any valid reason to report the value defined in the device tree
>>>>>> here.
>>>>>
>>>>> Here's a valid reason :-)
>>>>>
>>>>> lparstat isn't the only possible consumer of the interface, and the
>>>>> 'ibm,partition-name' property and the dynamic system parameter clearly
>>>>> serve a common purpose. 'ibm,partition-name' is provided by qemu.
>>>>
>>>> If the hypervisor is not providing this value, this is not the goal of this
>>>> interface to fetch it from the device tree.
>>>>
>>>> Any consumer should be able to fall back on the device tree value, and
>>>> there is no added value to do such a trick in the kernel when it can be
>>>> done in the user space.
>>>
>>> There is value in imposing a level of abstraction so that the semantics
>>> are:
>>>
>>> * Report the name assigned to the guest by the hosting environment, if
>>>   available
>>>
>>> as opposed to
>>>
>>> * Return the string returned by a RTAS call to ibm,get-system-parameter
>>>   with token 55, if implemented
>>>
>>> The benefit is that consumers of lparcfg do not have to be coded with
>>> the knowledge that "if a partition_name= line is absent, the
>>> ibm,get-system-parameter RTAS call must have failed, so now I should
>>> read /sys/firmware/devicetree/base/ibm,partition_name." That's the sort
>>> of esoterica that is appropriate for the kernel to encapsulate.
>>>
>>> And I'd say the effort involved (falling back to a root node property
>>> lookup) is proportional to the benefit.
>>>
>>
>> I don't agree.
>> From the kernel point of view, I can't see any benefit, this is adding more
>> complexity to do in the kernel what can be done easily in user space.
> 
> Applying this logic, I don't see how adding this to lparcfg would be
> justified at all, because user space can already get at the parameter
> using the privileged rtas syscall. Publish it to unprivileged programs
> over D-Bus or something. That would minimize complexity for the kernel.

As you know, there is a need for unprivileged user to read that value.
Adding this to lparcfg solves this issue. Also, this makes it easy to read
from user space, without the need to get plumbing like D-Bus in the
picture. This is simple, working fine, and user space can easily make the
choice to read the DT value if needed. Please keep in mind that most of the
time the hypervisor is providing that value.