Hi Julien,
Apologies for the delay, I was pulled away for a bit.
On 2/5/24 11:13, Julien Grall wrote:
> Hi,
>
> On 04/02/2024 09:40, Peng Fan wrote:
>>
>>
>>> -----Original Message-----
>>> From: Julien Grall <jul...@xen.org>
>>> Sent: 2024年2月2日 17:20
>>> To: John Ernberg <john.ernb...@actia.se>; Stefano Stabellini
>>> <sstabell...@kernel.org>; Bertrand Marquis <bertrand.marq...@arm.com>;
>>> Michal Orzel <michal.or...@amd.com>; Volodymyr Babchuk
>>> <volodymyr_babc...@epam.com>; Peng Fan <peng....@nxp.com>
>>> Cc: Jonas Blixt <jonas.bl...@actia.se>; xen-devel@lists.xenproject.org
>>> Subject: Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue
>>>
>>> On 01/02/2024 16:17, John Ernberg wrote:
>>>> On 2/1/24 13:20, Julien Grall wrote:
>>>>>
>>>>>
>>>>> On 31/01/2024 15:32, John Ernberg wrote:
>>>>>> Hi Julien,
>>>>>
>>>>> Hi John,
>>>>>
>>>>>> On 1/31/24 13:22, Julien Grall wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 31/01/2024 11:50, John Ernberg wrote:
[ ... ]
>>>>>
>>>>>>> But even if we restrict to dom0, have you checked that none of the
>>>>>>> SMCs use buffers?
>>>>>> I haven't found any such instances in the Linux kernel where a
>>>>>> buffer is used. Adding a call filtering like suggested below
>>>>>> additions of such functions can be discovered and adapted for if they
>>> would show up later.
>>>>>>>
>>>>>>> Rather than providing a blanket forward, to me it sounds more like
>>>>>>> you want to provide an allowlist of the SMCs. This is more
>>>>>>> futureproof and avoid the risk to expose unsafe SMCs to any domain.
>>>>>>>
>>>>>>> For an example, you can have a look at the EEMI mediator for Xilinx.
>>>>>>
>>>>>> Ack. Do you prefer to see only on SMCCC service level or also on
>>>>>> function level? (a1 register, per description earlier)
>>>>>
>>>>> I am not sure. It will depend on whether it is correct to expose
>>>>> *all* the functions within a service level and they have the same
>>>>> format.
>>>>>
>>>>> If you can't guarantee that, then you will most likely need to
>>>>> allowlist at the function level.
>>>>>
>>>>> Also, do you have a spec in hand that would help to understand which
>>>>> service/function is implemented via those SMCs?
>>>>
>>>> I don't have the spec unfortunately, but I will add a filter on both
>>>> service and function for V2 and we'll take it from there.
>>>
>>> @Peng, do you have any specification you could share? How stable is the
>>> interface?
>>
>> No specification, the use is IMX_SIP_X in linux kernel source.
>>
>> Such as IMX_SIP_RTC, IMX_SIP_TIMER
>>
>> It is stable and no change, we only add new SIP macro if needed
>> and no change the meaning or reuse old SIP.
>
> Thanks for the answer. It is a bit unfortunate there are no
> specification, but at least they are stable.
>
> I have searched IMX_SIP in Linux, there doesn't seem many so we could
> allowlist them (see more below). Do you know if there are more necessary
> that are not yet upstreamed in Linux?
I took a dive into both upstream kernel and the vendor tree and found
the following list and for which SoCs they are applicable (Please
correct me if you can Peng)
0x82000006 IMX_SIP_BUSFREQ_CHANGE [unsure, probably not imx8]
0xC2000000 IMX_SIP_GPC [only imx8m series]
0xC2000001 IMX_SIP_CPUFREQ [only imx8q{x,m} series]
0xC2000002 IMX_SIP_SRTC / IMX_SIP_TIMER [only imx8q{x,m} series]
0xC2000004 IMX_SIP_DDR_DVFS [only imx8m series]
0xC2000005 IMX_SIP_RPROC / IMX_SIP_SRC [only imx8m series]
0xC2000006 IMX_SIP_GET_SOC_INFO [only imx8m series]
0xC2000008 IMX_SIP_NOC [only imx8m series]
0xC2000009 IMX_SIP_WAKEUP_SRC [only imx8q{x,m} series]
0xC200000B IMX_SIP_OTP_WRITE [only imx8q{x,m} series]
>
>
> Looking through the list, there are some that probably want a bit more
> discussion on whether we want to expose them:
> * IMX_SIP_CPUFREQ: Right now, dom0 is not aware of the full system.
> So it may take wrong decision.
The cpufreq function operates on the cluster level, it performs no error
checking so blocking it will be invisible to Linux. I don't know yet
what kind of impact that could have. Looking for hints about cpufreq in
Xen for ARM I found [1] and [2].
In our current setup we do not use cpufreq and it is stable enough for
our usecase.
From my point of view we can block it for now, and when cpufreq
functionality gets picked up again, we can revisit.
> * IMX_SIP_DDR_DVFS: Some operation seems to take the number of online
> CPUs. Dom0 doesn't know that
The iMX8Q{X,M} series comes with a system controller called the SCU,
running in a small M-core. This controller takes care of all the DDR
bits, so we do not need to care for these SoCs.
> * IMX_SIP_TIMER_*: This seems to be related to the watchdog.
> Shouldn't dom0 rely on the watchdog provided by Xen instead? So those
> call will be used by Xen.
That is indeed a watchdog SIP, and also for setting the SoC internal RTC
if it is being used.
I looked around if there was previous discussion and only really found [3].
Is the xen/xen/include/watchdog.h header meant to be for this kind of
watchdog support or is that more for the VM watchdog? Looking at the x86
ACPI NMI watchdog it seems like the former, but I have never worked with
x86 nor ACPI.
Currently forwarding it to Dom0 has not caused any watchdog resets with
our watchdog timeout settings, our specific Dom0 setup and VM count.
For the remaining bits:
The wakeup src is not in the upstream kernel yet and related to system
resume from suspend which isn't supported in Xen on ARM yet - so this we
can block safely.
The OTP write is fuse programming in the SoC fuse banks, and should
probably be allowed but reserved for Dom0, as this can set fuses that
affects the SoC boot.
>
> Also, what happen if we don't expose those SMC to dom0?
>
> Cheers,
>
[1]:
https://lore.kernel.org/xen-devel/1510247421-24094-1-git-send-email-olekst...@gmail.com/
[2]:
https://www.slideshare.net/xen_com_mgr/xpdds18-cpufreq-in-xen-on-arm-oleksandr-tyshchenko-epam-systems
[3]: https://xen-users.narkive.com/ISXnlmB0/watchdog-support-in-xen
Thanks! // John Ernberg
