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=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham 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 A0E5FC433DB for ; Tue, 23 Mar 2021 19:51:15 +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 E23A2619C1 for ; Tue, 23 Mar 2021 19:51:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E23A2619C1 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]:60532 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lOn3Z-0006LV-PM for qemu-devel@archiver.kernel.org; Tue, 23 Mar 2021 15:51:13 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:60616) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lOmmb-0000aA-GK for qemu-devel@nongnu.org; Tue, 23 Mar 2021 15:33:43 -0400 Received: from mx2.suse.de ([195.135.220.15]:45422) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lOmmX-0005Hl-74 for qemu-devel@nongnu.org; Tue, 23 Mar 2021 15:33:39 -0400 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 0618DACBF; Tue, 23 Mar 2021 19:33:36 +0000 (UTC) Subject: Re: [RFC v10 39/49] target/arm: add tcg cpu accel class To: =?UTF-8?Q?Alex_Benn=c3=a9e?= References: <20210322140206.9513-1-cfontana@suse.de> <20210322140206.9513-40-cfontana@suse.de> <87h7l1isb0.fsf@linaro.org> From: Claudio Fontana Message-ID: <632c5af8-364f-8d61-d512-6c28fcf41fbe@suse.de> Date: Tue, 23 Mar 2021 20:33:35 +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: <87h7l1isb0.fsf@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit 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 , Eduardo Habkost , Richard Henderson , qemu-devel@nongnu.org, Roman Bolshakov , Paolo Bonzini , =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 3/23/21 8:26 PM, Alex Bennée wrote: > > Claudio Fontana writes: > >> move init, realizefn and reset code into it. > > w.r.t my testing this is fingered by bisect for causing: > > Running test qtest-aarch64/migration-test > ERROR qtest-aarch64/migration-test - Bail out! ERROR:../../target/arm/tcg/tcg-cpu.c:233:tcg_arm_init_accel_cpu: assertion failed: (object_class_by_name(ACCEL_CPU_NAME("tcg")) == OBJECT_CLASS(accel_cpu)) > Broken pipe > make: *** [Makefile.mtest:272: run-test-32] Error 1 > > The bisect log: > > git bisect start > # bad: [474677f44c0d581381db57ff6030a6553f16db95] XXX target/arm: experiment refactoring cpu "max" > git bisect bad 474677f44c0d581381db57ff6030a6553f16db95 > # good: [5ca634afcf83215a9a54ca6e66032325b5ffb5f6] Merge remote-tracking branch 'remotes/philmd/tags/sdmmc-20210322' into staging > git bisect good 5ca634afcf83215a9a54ca6e66032325b5ffb5f6 > # good: [bf31455e28246f6b9b732dc56f89e61895f6f4f0] i386: split svm_helper into sysemu and stub-only user > git bisect good bf31455e28246f6b9b732dc56f89e61895f6f4f0 > # good: [903f6e1bdc20939d023d6b577875370023bcac5f] target/arm: move arm_cpu_list to common_cpu > git bisect good 903f6e1bdc20939d023d6b577875370023bcac5f > # good: [1999c2ce6cc82200a39f6e41041f304eba5d4e7e] tests: device-introspect-test: cope with ARM TCG-only devices > git bisect good 1999c2ce6cc82200a39f6e41041f304eba5d4e7e > # bad: [fa37cc7fbabcf4de1cc963530c75b0ea7a73139a] target/arm: cpu-sve: new module > git bisect bad fa37cc7fbabcf4de1cc963530c75b0ea7a73139a > # good: [d0c4b7a7d2aa12537a06527692f10066b7acbed9] target/arm: create kvm cpu accel class > git bisect good d0c4b7a7d2aa12537a06527692f10066b7acbed9 > # bad: [3288c8a8cfc8dd9999e7b5908eaebb561f0169eb] target/arm: add tcg cpu accel class > git bisect bad 3288c8a8cfc8dd9999e7b5908eaebb561f0169eb > # good: [683b948d40bc81f1eaed27bef8631ab3f8a937d6] target/arm: move kvm post init initialization to kvm cpu accel > git bisect good 683b948d40bc81f1eaed27bef8631ab3f8a937d6 > # first bad commit: [3288c8a8cfc8dd9999e7b5908eaebb561f0169eb] target/arm: add tcg cpu accel class Sorry you did all this work, yes, it is a known issue that should be working in RFC v11. I completely forgot about the "all" build in this instance (the build with both kvm and tcg). Ciao, Claudio > > 3288c8a8cfc8dd9999e7b5908eaebb561f0169eb is the first bad commit > commit 3288c8a8cfc8dd9999e7b5908eaebb561f0169eb > Author: Claudio Fontana > Date: Mon Mar 22 15:01:56 2021 +0100 > > target/arm: add tcg cpu accel class > > move init, realizefn and reset code into it. > > Signed-off-by: Claudio Fontana > Cc: Paolo Bonzini > Message-Id: <20210322140206.9513-40-cfontana@suse.de> > > target/arm/cpu.c | 44 +++---------------------------- > target/arm/tcg/sysemu/tcg-cpu.c | 27 +++++++++++++++++++ > target/arm/tcg/tcg-cpu-models.c | 11 +++++--- > target/arm/tcg/tcg-cpu.c | 57 +++++++++++++++++++++++++++++++++++++++-- > target/arm/tcg/tcg-cpu.h | 4 ++- > 5 files changed, 96 insertions(+), 47 deletions(-) > bisect run success > > >> >> Signed-off-by: Claudio Fontana >> Cc: Paolo Bonzini >> --- >> target/arm/tcg/tcg-cpu.h | 4 ++- >> target/arm/cpu.c | 44 ++----------------------- >> target/arm/tcg/sysemu/tcg-cpu.c | 27 ++++++++++++++++ >> target/arm/tcg/tcg-cpu-models.c | 11 +++++-- >> target/arm/tcg/tcg-cpu.c | 57 +++++++++++++++++++++++++++++++-- >> 5 files changed, 96 insertions(+), 47 deletions(-) >> >> diff --git a/target/arm/tcg/tcg-cpu.h b/target/arm/tcg/tcg-cpu.h >> index d93c6a6749..dd08587949 100644 >> --- a/target/arm/tcg/tcg-cpu.h >> +++ b/target/arm/tcg/tcg-cpu.h >> @@ -22,15 +22,17 @@ >> >> #include "cpu.h" >> #include "hw/core/tcg-cpu-ops.h" >> +#include "hw/core/accel-cpu.h" >> >> void arm_cpu_synchronize_from_tb(CPUState *cs, >> const TranslationBlock *tb); >> >> -extern struct TCGCPUOps arm_tcg_ops; >> +void tcg_arm_init_accel_cpu(AccelCPUClass *accel_cpu, CPUClass *cc); >> >> #ifndef CONFIG_USER_ONLY >> /* Do semihosting call and set the appropriate return value. */ >> void tcg_handle_semihosting(CPUState *cs); >> +bool tcg_cpu_realizefn(CPUState *cs, Error **errp); >> >> #endif /* !CONFIG_USER_ONLY */ >> >> diff --git a/target/arm/cpu.c b/target/arm/cpu.c >> index 5e0f6bd01d..9248e096df 100644 >> --- a/target/arm/cpu.c >> +++ b/target/arm/cpu.c >> @@ -410,12 +410,6 @@ static void arm_cpu_reset(DeviceState *dev) >> &env->vfp.fp_status_f16); >> set_float_detect_tininess(float_tininess_before_rounding, >> &env->vfp.standard_fp_status_f16); >> - >> - if (tcg_enabled()) { >> - hw_breakpoint_update_all(cpu); >> - hw_watchpoint_update_all(cpu); >> - arm_rebuild_hflags(env); >> - } >> } >> >> void arm_cpu_update_virq(ARMCPU *cpu) >> @@ -576,10 +570,6 @@ static void arm_cpu_initfn(Object *obj) >> cpu->dtb_compatible = "qemu,unknown"; >> cpu->psci_version = 1; /* By default assume PSCI v0.1 */ >> cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE; >> - >> - if (tcg_enabled()) { >> - cpu->psci_version = 2; /* TCG implements PSCI 0.2 */ >> - } >> } >> >> static Property arm_cpu_gt_cntfrq_property = >> @@ -868,34 +858,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) >> Error *local_err = NULL; >> bool no_aa32 = false; >> >> - /* >> - * If we needed to query the host kernel for the CPU features >> - * then it's possible that might have failed in the initfn, but >> - * this is the first point where we can report it. >> - */ >> - if (cpu->host_cpu_probe_failed) { >> - error_setg(errp, "The 'host' CPU type can only be used with KVM"); >> - return; >> - } >> - >> -#ifndef CONFIG_USER_ONLY >> - /* The NVIC and M-profile CPU are two halves of a single piece of >> - * hardware; trying to use one without the other is a command line >> - * error and will result in segfaults if not caught here. >> - */ >> - if (arm_feature(env, ARM_FEATURE_M)) { >> - if (!env->nvic) { >> - error_setg(errp, "This board cannot be used with Cortex-M CPUs"); >> - return; >> - } >> - } else { >> - if (env->nvic) { >> - error_setg(errp, "This board can only be used with Cortex-M CPUs"); >> - return; >> - } >> - } >> - >> -#ifdef CONFIG_TCG >> +#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY) >> { >> uint64_t scale; >> >> @@ -921,8 +884,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) >> cpu->gt_timer[GTIMER_HYPVIRT] = timer_new(QEMU_CLOCK_VIRTUAL, scale, >> arm_gt_hvtimer_cb, cpu); >> } >> -#endif /* CONFIG_TCG */ >> -#endif /* !CONFIG_USER_ONLY */ >> +#endif /* CONFIG_TCG && !CONFIG_USER_ONLY */ >> >> cpu_exec_realizefn(cs, &local_err); >> if (local_err != NULL) { >> @@ -1458,7 +1420,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data) >> cc->disas_set_info = arm_disas_set_info; >> >> #ifdef CONFIG_TCG >> - cc->tcg_ops = &arm_tcg_ops; >> + cc->init_accel_cpu = tcg_arm_init_accel_cpu; >> #endif /* CONFIG_TCG */ >> >> arm32_cpu_class_init(oc, data); >> diff --git a/target/arm/tcg/sysemu/tcg-cpu.c b/target/arm/tcg/sysemu/tcg-cpu.c >> index 327b2a5073..115ac523dc 100644 >> --- a/target/arm/tcg/sysemu/tcg-cpu.c >> +++ b/target/arm/tcg/sysemu/tcg-cpu.c >> @@ -19,10 +19,13 @@ >> */ >> >> #include "qemu/osdep.h" >> +#include "qapi/error.h" >> +#include "qemu/timer.h" >> #include "cpu.h" >> #include "semihosting/common-semi.h" >> #include "qemu/log.h" >> #include "tcg/tcg-cpu.h" >> +#include "internals.h" >> >> /* >> * Do semihosting call and set the appropriate return value. All the >> @@ -50,3 +53,27 @@ void tcg_handle_semihosting(CPUState *cs) >> env->regs[15] += env->thumb ? 2 : 4; >> } >> } >> + >> +bool tcg_cpu_realizefn(CPUState *cs, Error **errp) >> +{ >> + ARMCPU *cpu = ARM_CPU(cs); >> + CPUARMState *env = &cpu->env; >> + >> + /* >> + * The NVIC and M-profile CPU are two halves of a single piece of >> + * hardware; trying to use one without the other is a command line >> + * error and will result in segfaults if not caught here. >> + */ >> + if (arm_feature(env, ARM_FEATURE_M)) { >> + if (!env->nvic) { >> + error_setg(errp, "This board cannot be used with Cortex-M CPUs"); >> + return false; >> + } >> + } else { >> + if (env->nvic) { >> + error_setg(errp, "This board can only be used with Cortex-M CPUs"); >> + return false; >> + } >> + } >> + return true; >> +} >> diff --git a/target/arm/tcg/tcg-cpu-models.c b/target/arm/tcg/tcg-cpu-models.c >> index 16ab5d5364..2f44fd1b41 100644 >> --- a/target/arm/tcg/tcg-cpu-models.c >> +++ b/target/arm/tcg/tcg-cpu-models.c >> @@ -844,15 +844,20 @@ static struct TCGCPUOps arm_v7m_tcg_ops = { >> #endif /* !CONFIG_USER_ONLY */ >> }; >> >> +static void arm_v7m_init_accel_cpu(AccelCPUClass *accel_cpu, CPUClass *cc) >> +{ >> + g_assert(object_class_by_name(ACCEL_CPU_NAME("tcg")) == OBJECT_CLASS(accel_cpu)); >> + >> + cc->tcg_ops = &arm_v7m_tcg_ops; >> +} >> + >> static void arm_v7m_class_init(ObjectClass *oc, void *data) >> { >> ARMCPUClass *acc = ARM_CPU_CLASS(oc); >> CPUClass *cc = CPU_CLASS(oc); >> >> acc->info = data; >> -#ifdef CONFIG_TCG >> - cc->tcg_ops = &arm_v7m_tcg_ops; >> -#endif /* CONFIG_TCG */ >> + cc->init_accel_cpu = arm_v7m_init_accel_cpu; >> >> cc->gdb_core_xml_file = "arm-m-profile.xml"; >> } >> diff --git a/target/arm/tcg/tcg-cpu.c b/target/arm/tcg/tcg-cpu.c >> index 9fd996d908..d6c3a0ba41 100644 >> --- a/target/arm/tcg/tcg-cpu.c >> +++ b/target/arm/tcg/tcg-cpu.c >> @@ -20,8 +20,8 @@ >> >> #include "qemu/osdep.h" >> #include "cpu.h" >> +#include "qapi/error.h" >> #include "tcg-cpu.h" >> -#include "hw/core/tcg-cpu-ops.h" >> #include "cpregs.h" >> #include "internals.h" >> #include "exec/exec-all.h" >> @@ -212,7 +212,7 @@ static bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request) >> return true; >> } >> >> -struct TCGCPUOps arm_tcg_ops = { >> +static struct TCGCPUOps arm_tcg_ops = { >> .initialize = arm_translate_init, >> .synchronize_from_tb = arm_cpu_synchronize_from_tb, >> .cpu_exec_interrupt = arm_cpu_exec_interrupt, >> @@ -227,3 +227,56 @@ struct TCGCPUOps arm_tcg_ops = { >> .debug_check_watchpoint = arm_debug_check_watchpoint, >> #endif /* !CONFIG_USER_ONLY */ >> }; >> + >> +void tcg_arm_init_accel_cpu(AccelCPUClass *accel_cpu, CPUClass *cc) >> +{ >> + g_assert(object_class_by_name(ACCEL_CPU_NAME("tcg")) == OBJECT_CLASS(accel_cpu)); >> + >> + cc->tcg_ops = &arm_tcg_ops; >> +} >> + >> +static void tcg_cpu_instance_init(CPUState *cs) >> +{ >> + ARMCPU *cpu = ARM_CPU(cs); >> + >> + /* >> + * this would be the place to move TCG-specific props >> + * in future refactoring of cpu properties. >> + */ >> + >> + cpu->psci_version = 2; /* TCG implements PSCI 0.2 */ >> +} >> + >> +static void tcg_cpu_reset(CPUState *cs) >> +{ >> + ARMCPU *cpu = ARM_CPU(cs); >> + CPUARMState *env = &cpu->env; >> + >> + hw_breakpoint_update_all(cpu); >> + hw_watchpoint_update_all(cpu); >> + arm_rebuild_hflags(env); >> +} >> + >> +static void tcg_cpu_accel_class_init(ObjectClass *oc, void *data) >> +{ >> + AccelCPUClass *acc = ACCEL_CPU_CLASS(oc); >> + >> +#ifndef CONFIG_USER_ONLY >> + acc->cpu_realizefn = tcg_cpu_realizefn; >> +#endif /* CONFIG_USER_ONLY */ >> + >> + acc->cpu_instance_init = tcg_cpu_instance_init; >> + acc->cpu_reset = tcg_cpu_reset; >> +} >> +static const TypeInfo tcg_cpu_accel_type_info = { >> + .name = ACCEL_CPU_NAME("tcg"), >> + >> + .parent = TYPE_ACCEL_CPU, >> + .class_init = tcg_cpu_accel_class_init, >> + .abstract = true, >> +}; >> +static void tcg_cpu_accel_register_types(void) >> +{ >> + type_register_static(&tcg_cpu_accel_type_info); >> +} >> +type_init(tcg_cpu_accel_register_types); > >