Hi Michal, Julien, > On Oct 11, 2023, at 14:54, Michal Orzel <[email protected]> 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 <[email protected]> >>> >>> Acked-by: Julien Grall <[email protected]> >> >> 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. > However, I find it useful, so I will leave it up to Henry to decide if we > want that or not. > > 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 <[email protected]> Kind regards, Henry > > ~Michal
