From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 30978C4361B for ; Fri, 18 Dec 2020 00:09:35 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7CA2423A57 for ; Fri, 18 Dec 2020 00:09:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7CA2423A57 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:41798 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kq3Kv-0006hk-Ao for qemu-devel@archiver.kernel.org; Thu, 17 Dec 2020 19:09:33 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:44732) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kq3JA-0006H6-Ba for qemu-devel@nongnu.org; Thu, 17 Dec 2020 19:07:44 -0500 Received: from mx2.suse.de ([195.135.220.15]:46322) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kq3J1-0006i6-Oh for qemu-devel@nongnu.org; Thu, 17 Dec 2020 19:07:44 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id E7209AD5A; Fri, 18 Dec 2020 00:07:33 +0000 (UTC) Subject: Re: [PULL 07/17] i386: move hyperv_vendor_id initialization to x86_cpu_realizefn() To: Eduardo Habkost References: <20201217184620.3945917-1-ehabkost@redhat.com> <20201217184620.3945917-8-ehabkost@redhat.com> <20201217225313.GO3140057@habkost.net> <5deb513d-336f-fadb-a15f-75f51e2211ed@suse.de> <20201217234702.GP3140057@habkost.net> From: Claudio Fontana Message-ID: <7889aaaf-06ca-e442-384e-9dd183e87681@suse.de> Date: Fri, 18 Dec 2020 01:07:33 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <20201217234702.GP3140057@habkost.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=195.135.220.15; envelope-from=cfontana@suse.de; helo=mx2.suse.de X-Spam_score_int: -41 X-Spam_score: -4.2 X-Spam_bar: ---- X-Spam_report: (-4.2 / 5.0 requ) BAYES_00=-1.9, NICE_REPLY_A=-0.001, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Maydell , qemu-devel@nongnu.org, Paolo Bonzini , Vitaly Kuznetsov , =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , Richard Henderson Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 12/18/20 12:47 AM, Eduardo Habkost wrote: > On Fri, Dec 18, 2020 at 12:34:46AM +0100, Claudio Fontana wrote: >> On 12/17/20 11:53 PM, Eduardo Habkost wrote: >>> On Thu, Dec 17, 2020 at 11:33:57PM +0100, Claudio Fontana wrote: >>>> Hello all, >>>> >>>> On 12/17/20 7:46 PM, Eduardo Habkost wrote: >>>>> From: Vitaly Kuznetsov >>>>> >>>>> As a preparation to expanding Hyper-V CPU features early, move >>>>> hyperv_vendor_id initialization to x86_cpu_realizefn(). Introduce >>>>> x86_cpu_hyperv_realize() to not not pollute x86_cpu_realizefn() >>>>> itself. >>>> >>>> this seems to fit very well the ongoing work on separating accelerator specific realize functions; >>>> >>>> related to the previous discussions about the class hierarchies, >>>> do you think that we should have a separate class in target/i386/kvm/ for a hyperv variant of the kvm-cpu.c? >>>> >>>> Should it be a separate class or a subclass of "kvm-accel-x86_64-cpu" ? >>> >>> I don't see how a separate QOM class for Hyper-V would be helpful >>> here. What would be the problem you are trying to solve in this >>> case? >> >> there is now a call to accelerator specific code x86_hyperv_cpu_realize just before cpu_exec_realize, >> tying the generic target/i386/cpu.c code to kvm/hyperv-specific accel initialization. >> >> if this is just a feature of the kvm accel, maybe I should just move all to kvm-cpu.c and that's it. > > That would make sense. If we decide this is a KVM-specific > feature, this code can be moved to kvm_cpu_realizefn(), provided > by the kvm-accel-x86_64-cpu class added by your series. > > However, I'm not sure we can say this is a KVM-specific feature. > The feature is currently only supported by the KVM accelerator, > but I'd say it is a generic feature. > Maybe in the future it will be a generic feature, and we can export it the right way? It will be super-easy if the feature is well contained. Until it really is generic though, should it really appear in the middle of x86_cpu_realizefn? currently the generic code in target/i386/cpu.c contains an unconditional call to: x86_cpu_hyperv_realize(cpu); before the call to cpu_exec_realizefn(). the function is defined in this very same file before as: static void x86_cpu_hyperv_realize(X86CPU *cpu) { ... } with a bunch of initializations of hyperv specific data, for example cpu->hyperv_interface_id, cpu->hyperv_vendor_id, cpu->hyperv_version_id. That data is ever only used in kvm/kvm.c, which is built conditionally under CONFIG_KVM. So the existing situation is fairly inconsistent in my view, at least for how things are now, and in principle the extra code of the initializations for hyperv should never be executed if !CONFIG_KVM. Thanks, Ciao, Claudio