Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757483AbcDHHN0 (ORCPT ); Fri, 8 Apr 2016 03:13:26 -0400 Received: from mx2.suse.de ([195.135.220.15]:36976 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753735AbcDHHNY (ORCPT ); Fri, 8 Apr 2016 03:13:24 -0400 Subject: Re: [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk To: "Luis R. Rodriguez" References: <1459987594-5434-1-git-send-email-mcgrof@kernel.org> <1459987594-5434-5-git-send-email-mcgrof@kernel.org> <570658DA.7060509@oracle.com> <20160408003207.GN1990@wotan.suse.de> <57073F0F.400@suse.com> <57075201.5080207@suse.com> Cc: Boris Ostrovsky , Borislav Petkov , "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , Rusty Russell , X86 ML , "linux-kernel@vger.kernel.org" , Andy Lutomirski , David Vrabel , Konrad Rzeszutek Wilk , "xen-devel@lists.xensource.com" , lguest@lists.ozlabs.org, Andy Shevchenko , Joey Lee , Gary Lin , Matt Fleming , Andrew Cooper , "Rafael J. Wysocki" , Len Brown , "Moore, Robert" , Lv Zheng , Toshi Kani , ACPI Devel Maling List , kozerkov@parallels.com, Josh Triplett , Joerg Roedel From: Juergen Gross Message-ID: <57075A0F.2020303@suse.com> Date: Fri, 8 Apr 2016 09:13:19 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3355 Lines: 81 On 08/04/16 08:56, Luis R. Rodriguez wrote: > On Thu, Apr 7, 2016 at 11:38 PM, Juergen Gross wrote: >> On 08/04/16 08:29, Luis R. Rodriguez wrote: >>> On Thu, Apr 7, 2016 at 10:18 PM, Juergen Gross wrote: >>>> On 08/04/16 02:32, Luis R. Rodriguez wrote: >>>>> This highlights a semantic gap issue. From a quick cursory review, I think >>>>> we can address this temporarily by just using a check: >>>>> >>>>> void __init x86_early_init_platform_quirks(void) >>>>> { >>>>> x86_platform.legacy.rtc = 1; >>>>> >>>>> switch (boot_params.hdr.hardware_subarch) { >>>>> case X86_SUBARCH_XEN: >>>>> case X86_SUBARCH_LGUEST: >>>>> case X86_SUBARCH_INTEL_MID: >>>>> - x86_platform.legacy.rtc = 0; >>>>> + if (x86_init.mpparse.get_smp_config != x86_init_uint_noop) >>>>> + x86_platform.legacy.rtc = 0; >>>> >>>> No! Why don't you just use the explicit test xen_initial_domain() ? >>> >>> Because we don't want to sprinkle Xen specific code outside of Xen >>> code. What do you think about the second possibility I listed? >>> Otherwise, any other ideas? >> >> Don't try to guess. > > I can only do that given there is nothing at all to tell me what to > expect here with regards to RTC on Xen guest, if there is some > documentation that could help with that please let me know. Only Xen inernals. :-) > >> In case you don't want to inject Xen internals here, just call a Xen >> function to either return the correct value, or to set all structure >> elements correctly. > > I like the later as an option, in case there are further hardware > subarch specific quirks which require internal logistics. What do > others think? > >> Thinking more about it: why not do that for all the subarchs? > > I originally had went with that approach, but Ingo made the point that > it would be best to instead move all quirk settings into one place. > That lets a reader easily tell what is going on in one place, it also > compartmentalizes the hardware subarch uses. Okay. Another idea (not sure whether this is really a good one): Add X86_SUBARCH_XEN_DOM0. As hardware_subarch is 32 bits wide I don't think the number of subarchs is a scarce resource. :-) I'd expect other quirks in future might have different settings for domU and dom0, too. >> You'd >> have the specific settings where they belong: in a subarch specific >> source. Just do the default settings in x86_early_init_platform_quirks() >> and let the subarch functions set the non-default values. > > This is a rather different approach than what I had originally tried. > Bike shed thing -- someone just has to decide. > > Left up to me, I kind of really like centralizing the quirk settings > in one place approach as it means a reader can easily tell what's > going on regardless of platform in one place for odd settings. I > prefer this given that we *already* have the semantics over hardware > subarch in a generalized fashion. We *do not* have semantics for dom0 > Vs domU -- if such a notion is generic to other virtualization That's not carved in stone - see above. :-) > environments it deserves consideration to new semantics to deal with > that, otherwise the callback for handling further quirks is best, but > I'd also highly discourage such callback to be used. Juergen