Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757575AbcDHG5F (ORCPT ); Fri, 8 Apr 2016 02:57:05 -0400 Received: from mail.kernel.org ([198.145.29.136]:39669 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757159AbcDHG5B (ORCPT ); Fri, 8 Apr 2016 02:57:01 -0400 MIME-Version: 1.0 In-Reply-To: <57075201.5080207@suse.com> 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> From: "Luis R. Rodriguez" Date: Thu, 7 Apr 2016 23:56:31 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk To: Juergen Gross 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 Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2865 Lines: 65 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. > 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. > 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 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. Luis