Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753369AbbL2MvT (ORCPT ); Tue, 29 Dec 2015 07:51:19 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:42619 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751756AbbL2MvP (ORCPT ); Tue, 29 Dec 2015 07:51:15 -0500 Subject: Re: [PATCH RFC 1/3] x86/pvclock: add setter for pvclock_pvti_cpu0_va To: Andy Lutomirski References: <1451339557-24473-1-git-send-email-joao.m.martins@oracle.com> <1451339557-24473-2-git-send-email-joao.m.martins@oracle.com> Cc: "linux-kernel@vger.kernel.org" , kvm list , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , X86 ML , Gleb Natapov , Paolo Bonzini , Andy Lutomirski , Borislav Petkov , Marcelo Tosatti , xen-devel From: Joao Martins Message-ID: <568281AC.9010900@oracle.com> Date: Tue, 29 Dec 2015 12:50:52 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3033 Lines: 70 On 12/28/2015 11:45 PM, Andy Lutomirski wrote: > On Mon, Dec 28, 2015 at 1:52 PM, Joao Martins wrote: >> Right now there is only a pvclock_pvti_cpu0_va() which is defined on >> kvmclock since: >> >> commit dac16fba6fc5 >> ("x86/vdso: Get pvclock data from the vvar VMA instead of the fixmap") >> >> The only user of this interface so far is kvm. This commit adds a setter >> function for the pvti page and moves pvclock_pvti_cpu0_va to pvclock, which >> is a more generic place to have it; and would allow other PV clocksources >> to use it, such as Xen. >> > >> + >> +void pvclock_set_pvti_cpu0_va(struct pvclock_vsyscall_time_info *pvti) >> +{ >> + pvti_cpu0_va = pvti; >> +} > > IMO this either wants to be __init or wants a > WARN_ON(vclock_was_used(VCLOCK_PVCLOCK)). The latter hasn't landed in > -tip yet, but I think it'll land next week unless the merge window > opens early. OK, I will add those two once it lands in -tip. I had a silly mistake in this patch as I bindly ommited the parameter name to keep checkpatch happy, but didn't compile check when built without PARAVIRT. Apologies for that and will fix that also on the next version. > > It may pay to actually separate out the kvm-clock clocksource and > rename it rather than partially duplicating it, assuming the result > wouldn't be messy. > Not sure if I follow but I moved out pvclock_pvti_cpu0_va from kvm-clock or do you mean to separate out kvm-clock in it's enterity, or something else within kvm-clock is that is common to both (such as kvm_setup_vsyscall_timeinfo) ? > Can you CC me on the rest of the series for new versions? > Sure! Thanks for the prompt reply. > BTW, since this seems to require hypervisor changes to be useful, it > might make sense to rethink the interface a bit. Are you actually > planning to support per-cpu pvti for this in any useful way? If not, > I think that this would work a whole lot better and be considerably > less code if you had a single global pvti that lived in > hypervisor-allocated memory instead of an array that lives in guest > memory. I'd be happy to discuss next week in more detail (currently > on vacation). Initially I had this series using per-cpu pvti's based on Linux 4.4 but since that was removed in favor of vdso using solely cpu0 pvti, then I ended up just registering the cpu 0 page. I don't intend to add per-cpu pvti's since it would only be used for this case: (unless the reviewers think it should be done) meaning I would register pvti's for the other CPUs without having them used. Having a global pvti as you suggest it would get a lot simpler for the guest, but I guess this would only work assuming PVCLOCK_TSC_STABLE_BIT is there? Looking forward to discuss it next week. Joao > > --Andy > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/