Re: [RFC PATCH 00/31] CPUFreq on ARM

[Jassi Brar <jaswinder.singh@linaro.org>]

On 15 November 2017 at 18:58, Andre Przywara <andre.przywara@linaro.org> wrote:
> Hi,
>
> On 15/11/17 03:03, Jassi Brar wrote:
>> On 15 November 2017 at 02:16, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
>>> On Tue, Nov 14, 2017 at 12:49 PM, Andre Przywara
>>> <andre.przywara@linaro.org> wrote:
>>>
>>
>>>>>>> 3. Direct ported SCPI protocol, mailbox infrastructure and the ARM SMC triggered mailbox driver. All components except mailbox driver are in mainline Linux.
>>>>>>
>>>>>> Why do you actually need this mailbox framework?
>>>
>> It is unnecessary if you are always going to use one particular signal
>> mechanism, say SMC. However ...
>>
>>>>>> Actually I just
>>>>>> proposed the SMC driver the make it fit into the Linux framework. All we
>>>>>> actually need for SCPI is to write a simple command into some memory and
>>>>>> "press a button". I don't see a need to import the whole Linux
>>>>>> framework, especially as our mailbox usage is actually just a corner
>>>>>> case of the mailbox's capability (namely a "single-bit" doorbell).
>>>>>> The SMC use case is trivial to implement, and I believe using the Juno
>>>>>> mailbox is similarly simple, for instance.
>>>
>> ... Its going to be SMC and MHU now... and you talk about Rockchip as
>> well later. That becomes unwieldy.
>>
>>
>>>>
>>>>> Protocol relies on mailbox feature, so I ported mailbox too. I think,
>>>>> it would be much more easy for me to just add
>>>>> a few required commands handling with issuing SMC call and without any
>>>>> mailbox infrastructure involved.
>>>>> But, I want to show what is going on and what place these things come from.
>>>>
>>>> I appreciate that, but I think we already have enough "bloated" Linux +
>>>> glue code in Xen. And in particular the Linux mailbox framework is much
>>>> more powerful than we need for SCPI, so we have a lot of unneeded
>>>> functionality.
>>>
>> That is a painful misconception.
>> Mailbox api is designed to be (almost) as light weight as being
>> transparent. Please have a look at mbox_send_message() and see how
>> negligible overhead it adds for "SMC controller" that you compare
>> against here..... just integer manipulations protected by a spinlock.
>> Of course if your protocol needs async messaging, you pay the price
>> but only fair.
>
> Normally I would agree on importing some well designed code rather than
> hacking up something yourself.
>
> BUT: This is Xen, which is meant to be lean, micro-kernel like
> hypervisor. If we now add code from Linux, there must be a good
> rationale why we need it. And this is why we need to make sure that
> CPUFreq is really justified in the first place.
> So I am a bit wary that pulling some rather unrelated Linux *framework*
> into Xen bloats it up and introduces more burden to the trusted code
> base. With SCPI being the only user, this controller - client
> abstraction is not really needed. And to just trigger an interrupt on
> the SCP side we just need to:
>         writel(BIT(channel), base_addr + CPU_INTR_H_SET);
>
> I expect other mailboxes to be similarly simple.
> The only other code needed is some DT parsing.
>
> That being said I haven't look too closely how much code this actually
> pulls in, it is just my gut feeling that it's a bit over the top,
> conceptually.
>
Please do have a look and let me know how it drags the SCP down.

Thanks

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel