[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] xen/arm: Validate generic timer frequency
On 11/10/2023 08:01, Henry Wang wrote:
Hi Michal, Julien,
On Oct 11, 2023, at 14:54, Michal Orzel <michal.orzel@xxxxxxx> wrote:
Hi Julien, Henry,
On 10/10/2023 16:48, Julien Grall wrote:
(+ Henry)
Hi Michal,
On 29/09/2023 08:38, Julien Grall wrote:
Hi Michal,
On 28/09/2023 13:34, Michal Orzel wrote:
Generic timer dt node property "clock-frequency" (refer Linux dt binding
Documentation/devicetree/bindings/timer/arm,arch_timer.yaml) is used to
override the incorrect value set by firmware in CNTFRQ_EL0. If the value
of this property is 0 (i.e. by mistake), Xen would continue to work and
use the value from the sysreg instead. The logic is thus incorrect and
results in inconsistency when creating timer node for domUs:
- libxl domUs: clock_frequency member of struct xen_arch_domainconfig
is set to 0 and libxl ignores setting the "clock-frequency" property,
- dom0less domUs: "clock-frequency" property is taken from dt_host and
thus set to 0.
Property "clock-frequency" is used to override the value from sysreg,
so if it is also invalid, there is nothing we can do and we shall panic
to let user know about incorrect setting. Going even further, we operate
under assumption that the frequency must be at least 1KHz (i.e. cpu_khz
not 0) in order for Xen to boot. Therefore, introduce a new helper
validate_timer_frequency() to verify this assumption and use it to check
the frequency obtained either from dt property or from sysreg.
Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
Acked-by: Julien Grall <jgrall@xxxxxxxxxx>
Going through the list of pending patches, I noticed Henry wasn't CCed.
Is this patch intended for Xen 4.18? If so, can you provide some
reasoning why would want it?
Strictly speaking, lack of "for-4.18" prefix indicates that I did not
particularly aim for 4.18.
I have seen a mix. Hence why I asked in case this was expected. To avoid
confusion during the freeze period, I tend to also tag e-mail for the
next release (e.g. for-next/for-4.19). So the intention is clear.
Benefits:
- fixes the invalid logic the consequence of which might result in
inconsistency when booting
the same OS as libxl domU and dom0less domU.
- prevents spreading out the error condition (i.e. rate < 1KHZ) that can lead
to e.g. Xen inability to schedule,
by panicing with proper msg
Risks:
- early init code, no critical path, in case of an error, panic returned with
proper msg - no risks AFAICT
Thanks for the explanation, this looks like an improvement for the
current code to me so I am fine with including it in 4.18
Release-acked-by: Henry Wang <Henry.Wang@xxxxxxx>
Thanks. I will commit the patch later today.
Cheers,
--
Julien Grall
|
