[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 4/7] x86/time: introduce probing logic for the wallclock
On 03.09.2024 15:03, Roger Pau Monne wrote: > Adding such probing allows to clearly separate init vs runtime code, and to > place the probing logic into the init section for the CMOS case. Note both > the Xen shared_info page wallclock, and the EFI wallclock don't really have > any > probing-specific logic. The shared_info wallclock will always be there if > booted as a Xen guest, while the EFI_GET_TIME method probing relies on > checking > if it returns a value different than 0. > > The panic message printed when Xen is unable to find a viable wallclock source > has been adjusted slightly, I believe the printed guidance still provides the > same amount of information to the user. > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> Looks a little involved, but I'm largely fine with it; just a couple of more or less cosmetic remarks: > @@ -1329,28 +1338,13 @@ static bool cmos_probe(struct rtc_time *rtc_p, bool > cmos_rtc_probe) > return false; > } > > -static unsigned long get_cmos_time(void) > + > +static unsigned long cmos_read(void) > { > - unsigned long res; > struct rtc_time rtc; > - static bool __read_mostly cmos_rtc_probe; > - boolean_param("cmos-rtc-probe", cmos_rtc_probe); > + bool success = __get_cmos_time(&rtc); > > - if ( efi_enabled(EFI_RS) ) > - { > - res = efi_get_time(); > - if ( res ) > - return res; > - } > - > - if ( likely(!(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC)) ) > - cmos_rtc_probe = false; > - else if ( system_state < SYS_STATE_smp_boot && !cmos_rtc_probe ) > - panic("System with no CMOS RTC advertised must be booted from EFI" > - " (or with command line option \"cmos-rtc-probe\")\n"); > - > - if ( !cmos_probe(&rtc, cmos_rtc_probe) ) > - panic("No CMOS RTC found - system must be booted from EFI\n"); > + ASSERT(success); I'm not convinced of this assertion: It's either too much (compared to what we had so far) or not enough, considering the behavior ... > return mktime(rtc.year, rtc.mon, rtc.day, rtc.hour, rtc.min, rtc.sec); > } ... with a release build. > @@ -1533,12 +1527,82 @@ void rtc_guest_write(unsigned int port, unsigned int > data) > } > } > > -static unsigned long get_wallclock_time(void) > +static enum { > + WALLCLOCK_UNSET, > + WALLCLOCK_XEN, > + WALLCLOCK_CMOS, > + WALLCLOCK_EFI, > +} wallclock_source __ro_after_init; > + > +static const char *wallclock_type_to_string(void) __init ? > { > + switch ( wallclock_source ) > + { > + case WALLCLOCK_XEN: > + return "XEN"; > + > + case WALLCLOCK_CMOS: > + return "CMOS RTC"; > + > + case WALLCLOCK_EFI: > + return "EFI"; > + > + case WALLCLOCK_UNSET: > + return "UNSET"; > + } > + > + ASSERT_UNREACHABLE(); > + return ""; > +} > + > +static void __init probe_wallclock(void) > +{ > + ASSERT(wallclock_source == WALLCLOCK_UNSET); > + > if ( xen_guest ) > + { > + wallclock_source = WALLCLOCK_XEN; > + return; > + } > + if ( efi_enabled(EFI_RS) && efi_get_time() ) > + { > + wallclock_source = WALLCLOCK_EFI; > + return; > + } > + if ( cmos_probe() ) > + { > + wallclock_source = WALLCLOCK_CMOS; > + return; > + } > + > + panic("No usable wallclock found, probed:%s%s%s\n%s", > + !cmos_rtc_probe && !efi_enabled(EFI_RS) ? " None" : "", > + cmos_rtc_probe ? " CMOS" : "", > + efi_enabled(EFI_RS) ? " EFI" : "", > + !cmos_rtc_probe ? "Try with command line option > \"cmos-rtc-probe\"\n" > + : !efi_enabled(EFI_RS) ? "System must be booted from EFI\n" : ""); This last argument is sufficiently complex that I think it is pretty important for the question marks and colons to respectively align with one another, even if this may mean one or two more lines of code. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |