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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 8851ACDB47E for ; Fri, 13 Oct 2023 18:34:23 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qrMyQ-0003Yx-O4; Fri, 13 Oct 2023 14:33:23 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qrMy3-0003Ty-HH; Fri, 13 Oct 2023 14:32:59 -0400 Received: from mail.ilande.co.uk ([2001:41c9:1:41f::167]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qrMxx-0003AB-Nq; Fri, 13 Oct 2023 14:32:59 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=ilande.co.uk; s=20220518; h=Subject:Content-Transfer-Encoding:Content-Type: In-Reply-To:From:References:Cc:To:MIME-Version:Date:Message-ID:Sender: Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender :Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=byuzCLLc3O3xLpRGuOu5GkIYJyADF9C8nBaG30TQQWs=; b=jDIkx7oBgzHmxmwYdSDHMb7NzH /xC0Tq5dWWaPivS9YK0TAdVwbnUH8B4u9OleOFfDZMEMWzczh3wKfd5uEjBMWuO5nFaqkGE0plKjy ycNYgSksOC4rU/vFlxCYGcTkY7L7yRj0BTCbInuivNyqC8CPIqS+pUsDcDNuQUz4fkfsq/V7zHqSU qVhI8iQkn/CjDD+W60u/7O4M3xNhb9hqH0Fg6GRG1bwuJOqvvM/+ZL6schvnSN6bVlktM1jCYyX0D CbEPvKm/weIKyt3hFjFr31L1bXQOnaimTuLCee/a7qJRgAxSUR9i841cVs1TSmEFrjcEUHBjjFBPX kPAZdif89hWoFYpPg2IreZtcJUQvejYhWUvVUTfSjehHUc378CwkEPQbnTyxWf8IuILu1foMOgGij 8wRsDv5i9SmQ1Uaf7rdYGW2OEojF+mEOyy6BqCQmaPJDAKvVQ2LTJ34Alqd4pEJeT1XPTVZV1Um6l pMxGB8GU84yd2lixq9inuUBetxEWRIi+e0Lz0DPM75Vv9192+l6QuQye5+hyGi/TtKkUCZHjSmqGH a1ZXhCF6Q2C/A2E3f1e7k/FLLGO5TgBCnLzK/bEqo2H4MN9SBuozmmK1KKsa8qOIShtKtpHXhpL1H hhaqno7Yx9y4AgvGSXCgwPmyvUpjmvnIHTL34c0LI=; Received: from [2a00:23c4:8baf:fd00:1033:b5d6:92a3:359f] by mail.ilande.co.uk with esmtpsa (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1qrMxn-0002rq-2E; Fri, 13 Oct 2023 19:32:47 +0100 Message-ID: Date: Fri, 13 Oct 2023 19:32:43 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: =?UTF-8?Q?Philippe_Mathieu-Daud=C3=A9?= , qemu-devel@nongnu.org Cc: David Gibson , qemu-ppc@nongnu.org, Nicholas Piggin , Harsh Prateek Bora , =?UTF-8?Q?C=C3=A9dric_Le_Goater?= , Daniel Henrique Barboza References: <20231013125630.95116-1-philmd@linaro.org> <20231013125630.95116-2-philmd@linaro.org> From: Mark Cave-Ayland Autocrypt: addr=mark.cave-ayland@ilande.co.uk; keydata= xsBNBFQJuzwBCADAYvxrwUh1p/PvUlNFwKosVtVHHplgWi5p29t58QlOUkceZG0DBYSNqk93 3JzBTbtd4JfFcSupo6MNNOrCzdCbCjZ64ik8ycaUOSzK2tKbeQLEXzXoaDL1Y7vuVO7nL9bG E5Ru3wkhCFc7SkoypIoAUqz8EtiB6T89/D9TDEyjdXUacc53R5gu8wEWiMg5MQQuGwzbQy9n PFI+mXC7AaEUqBVc2lBQVpAYXkN0EyqNNT12UfDLdxaxaFpUAE2pCa2LTyo5vn5hEW+i3VdN PkmjyPvL6DdY03fvC01PyY8zaw+UI94QqjlrDisHpUH40IUPpC/NB0LwzL2aQOMkzT2NABEB AAHNME1hcmsgQ2F2ZS1BeWxhbmQgPG1hcmsuY2F2ZS1heWxhbmRAaWxhbmRlLmNvLnVrPsLA eAQTAQIAIgUCVAm7PAIbAwYLCQgHAwIGFQgCCQoLBBYCAwECHgECF4AACgkQW8LFb64PMh9f NAgAuc3ObOEY8NbZko72AGrg2tWKdybcMVITxmcor4hb9155o/OWcA4IDbeATR6cfiDL/oxU mcmtXVgPqOwtW3NYAKr5g/FrZZ3uluQ2mtNYAyTFeALy8YF7N3yhs7LOcpbFP7tEbkSzoXNG z8iYMiYtKwttt40WaheWuRs0ZOLbs6yoczZBDhna3Nj0LA3GpeJKlaV03O4umjKJgACP1c/q T2Pkg+FCBHHFP454+waqojHp4OCBo6HyK+8I4wJRa9Z0EFqXIu8lTDYoggeX0Xd6bWeCFHK3 DhD0/Xi/kegSW33unsp8oVcM4kcFxTkpBgj39dB4KwAUznhTJR0zUHf63M7ATQRUCbs8AQgA y7kyevA4bpetM/EjtuqQX4U05MBhEz/2SFkX6IaGtTG2NNw5wbcAfhOIuNNBYbw6ExuaJ3um 2uLseHnudmvN4VSJ5Hfbd8rhqoMmmO71szgT/ZD9MEe2KHzBdmhmhxJdp+zQNivy215j6H27 14mbC2dia7ktwP1rxPIX1OOfQwPuqlkmYPuVwZP19S4EYnCELOrnJ0m56tZLn5Zj+1jZX9Co YbNLMa28qsktYJ4oU4jtn6V79H+/zpERZAHmH40IRXdR3hA+Ye7iC/ZpWzT2VSDlPbGY9Yja Sp7w2347L5G+LLbAfaVoejHlfy/msPeehUcuKjAdBLoEhSPYzzdvEQARAQABwsBfBBgBAgAJ BQJUCbs8AhsMAAoJEFvCxW+uDzIfabYIAJXmBepHJpvCPiMNEQJNJ2ZSzSjhic84LTMWMbJ+ opQgr5cb8SPQyyb508fc8b4uD8ejlF/cdbbBNktp3BXsHlO5BrmcABgxSP8HYYNsX0n9kERv NMToU0oiBuAaX7O/0K9+BW+3+PGMwiu5ml0cwDqljxfVN0dUBZnQ8kZpLsY+WDrIHmQWjtH+ Ir6VauZs5Gp25XLrL6bh/SL8aK0BX6y79m5nhfKI1/6qtzHAjtMAjqy8ChPvOqVVVqmGUzFg KPsrrIoklWcYHXPyMLj9afispPVR8e0tMKvxzFBWzrWX1mzljbBlnV2n8BIwVXWNbgwpHSsj imgcU9TTGC5qd9g= In-Reply-To: <20231013125630.95116-2-philmd@linaro.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-SA-Exim-Connect-IP: 2a00:23c4:8baf:fd00:1033:b5d6:92a3:359f X-SA-Exim-Mail-From: mark.cave-ayland@ilande.co.uk Subject: Re: [PATCH 1/7] hw/ppc/spapr: Restrict PPCTimebase structure declaration to sPAPR X-SA-Exim-Version: 4.2.1 (built Wed, 08 May 2019 21:11:16 +0000) X-SA-Exim-Scanned: Yes (on mail.ilande.co.uk) Received-SPF: pass client-ip=2001:41c9:1:41f::167; envelope-from=mark.cave-ayland@ilande.co.uk; helo=mail.ilande.co.uk X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, 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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org On 13/10/2023 13:56, Philippe Mathieu-Daudé wrote: > The PPCTimebase structure is only used by the sPAPR machine. > Move its declaration to "hw/ppc/spapr.h". > Move vmstate_ppc_timebase and the VMSTATE_PPC_TIMEBASE_V() > macro to hw/ppc/spapr.c, along with the timebase_foo() > migration helpers. > > Signed-off-by: Philippe Mathieu-Daudé > --- > include/hw/ppc/spapr.h | 6 +++ > target/ppc/cpu-qom.h | 22 -------- > hw/ppc/ppc.c | 107 ------------------------------------- > hw/ppc/spapr.c | 116 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 122 insertions(+), 129 deletions(-) > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index e91791a1a9..3cf9978cba 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -163,6 +163,12 @@ struct SpaprMachineClass { > SpaprIrq *irq; > }; > > +typedef struct PPCTimebase { > + uint64_t guest_timebase; > + int64_t time_of_the_day_ns; > + bool runstate_paused; > +} PPCTimebase; > + > #define WDT_MAX_WATCHDOGS 4 /* Maximum number of watchdog devices */ > > #define TYPE_SPAPR_WDT "spapr-wdt" > diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h > index be33786bd8..b5deef5ca5 100644 > --- a/target/ppc/cpu-qom.h > +++ b/target/ppc/cpu-qom.h > @@ -197,26 +197,4 @@ struct PowerPCCPUClass { > int (*check_pow)(CPUPPCState *env); > }; > > -#ifndef CONFIG_USER_ONLY > -typedef struct PPCTimebase { > - uint64_t guest_timebase; > - int64_t time_of_the_day_ns; > - bool runstate_paused; > -} PPCTimebase; > - > -extern const VMStateDescription vmstate_ppc_timebase; > - > -#define VMSTATE_PPC_TIMEBASE_V(_field, _state, _version) { \ > - .name = (stringify(_field)), \ > - .version_id = (_version), \ > - .size = sizeof(PPCTimebase), \ > - .vmsd = &vmstate_ppc_timebase, \ > - .flags = VMS_STRUCT, \ > - .offset = vmstate_offset_value(_state, _field, PPCTimebase), \ > -} > - > -void cpu_ppc_clock_vm_state_change(void *opaque, bool running, > - RunState state); > -#endif > - > #endif > diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c > index be167710a3..340cd6192f 100644 > --- a/hw/ppc/ppc.c > +++ b/hw/ppc/ppc.c > @@ -32,7 +32,6 @@ > #include "qemu/main-loop.h" > #include "qemu/error-report.h" > #include "sysemu/kvm.h" > -#include "sysemu/replay.h" > #include "sysemu/runstate.h" > #include "kvm_ppc.h" > #include "migration/vmstate.h" > @@ -967,112 +966,6 @@ void cpu_ppc_store_purr(CPUPPCState *env, uint64_t value) > _cpu_ppc_store_purr(env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), value); > } > > -static void timebase_save(PPCTimebase *tb) > -{ > - uint64_t ticks = cpu_get_host_ticks(); > - PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu); > - > - if (!first_ppc_cpu->env.tb_env) { > - error_report("No timebase object"); > - return; > - } > - > - if (replay_mode == REPLAY_MODE_NONE) { > - /* not used anymore, we keep it for compatibility */ > - tb->time_of_the_day_ns = qemu_clock_get_ns(QEMU_CLOCK_HOST); > - } else { > - /* simpler for record-replay to avoid this event, compat not needed */ > - tb->time_of_the_day_ns = 0; > - } > - > - /* > - * tb_offset is only expected to be changed by QEMU so > - * there is no need to update it from KVM here > - */ > - tb->guest_timebase = ticks + first_ppc_cpu->env.tb_env->tb_offset; > - > - tb->runstate_paused = > - runstate_check(RUN_STATE_PAUSED) || runstate_check(RUN_STATE_SAVE_VM); > -} > - > -static void timebase_load(PPCTimebase *tb) > -{ > - CPUState *cpu; > - PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu); > - int64_t tb_off_adj, tb_off; > - unsigned long freq; > - > - if (!first_ppc_cpu->env.tb_env) { > - error_report("No timebase object"); > - return; > - } > - > - freq = first_ppc_cpu->env.tb_env->tb_freq; > - > - tb_off_adj = tb->guest_timebase - cpu_get_host_ticks(); > - > - tb_off = first_ppc_cpu->env.tb_env->tb_offset; > - trace_ppc_tb_adjust(tb_off, tb_off_adj, tb_off_adj - tb_off, > - (tb_off_adj - tb_off) / freq); > - > - /* Set new offset to all CPUs */ > - CPU_FOREACH(cpu) { > - PowerPCCPU *pcpu = POWERPC_CPU(cpu); > - pcpu->env.tb_env->tb_offset = tb_off_adj; > - kvmppc_set_reg_tb_offset(pcpu, pcpu->env.tb_env->tb_offset); > - } > -} > - > -void cpu_ppc_clock_vm_state_change(void *opaque, bool running, > - RunState state) > -{ > - PPCTimebase *tb = opaque; > - > - if (running) { > - timebase_load(tb); > - } else { > - timebase_save(tb); > - } > -} > - > -/* > - * When migrating a running guest, read the clock just > - * before migration, so that the guest clock counts > - * during the events between: > - * > - * * vm_stop() > - * * > - * * pre_save() > - * > - * This reduces clock difference on migration from 5s > - * to 0.1s (when max_downtime == 5s), because sending the > - * final pages of memory (which happens between vm_stop() > - * and pre_save()) takes max_downtime. > - */ > -static int timebase_pre_save(void *opaque) > -{ > - PPCTimebase *tb = opaque; > - > - /* guest_timebase won't be overridden in case of paused guest or savevm */ > - if (!tb->runstate_paused) { > - timebase_save(tb); > - } > - > - return 0; > -} > - > -const VMStateDescription vmstate_ppc_timebase = { > - .name = "timebase", > - .version_id = 1, > - .minimum_version_id = 1, > - .pre_save = timebase_pre_save, > - .fields = (VMStateField []) { > - VMSTATE_UINT64(guest_timebase, PPCTimebase), > - VMSTATE_INT64(time_of_the_day_ns, PPCTimebase), > - VMSTATE_END_OF_LIST() > - }, > -}; > - > /* Set up (once) timebase frequency (in Hz) */ > void cpu_ppc_tb_init(CPUPPCState *env, uint32_t freq) > { > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index cb840676d3..fe8b425ffd 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -37,6 +37,7 @@ > #include "sysemu/numa.h" > #include "sysemu/qtest.h" > #include "sysemu/reset.h" > +#include "sysemu/replay.h" > #include "sysemu/runstate.h" > #include "qemu/log.h" > #include "hw/fw-path-provider.h" > @@ -1809,6 +1810,100 @@ static bool spapr_vga_init(PCIBus *pci_bus, Error **errp) > } > } > > +static void timebase_save(PPCTimebase *tb) > +{ > + uint64_t ticks = cpu_get_host_ticks(); > + PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu); > + > + if (!first_ppc_cpu->env.tb_env) { > + error_report("No timebase object"); > + return; > + } > + > + if (replay_mode == REPLAY_MODE_NONE) { > + /* not used anymore, we keep it for compatibility */ > + tb->time_of_the_day_ns = qemu_clock_get_ns(QEMU_CLOCK_HOST); > + } else { > + /* simpler for record-replay to avoid this event, compat not needed */ > + tb->time_of_the_day_ns = 0; > + } > + > + /* > + * tb_offset is only expected to be changed by QEMU so > + * there is no need to update it from KVM here > + */ > + tb->guest_timebase = ticks + first_ppc_cpu->env.tb_env->tb_offset; > + > + tb->runstate_paused = > + runstate_check(RUN_STATE_PAUSED) || runstate_check(RUN_STATE_SAVE_VM); > +} > + > +static void timebase_load(PPCTimebase *tb) > +{ > + CPUState *cpu; > + PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu); > + int64_t tb_off_adj, tb_off; > + unsigned long freq; > + > + if (!first_ppc_cpu->env.tb_env) { > + error_report("No timebase object"); > + return; > + } > + > + freq = first_ppc_cpu->env.tb_env->tb_freq; > + > + tb_off_adj = tb->guest_timebase - cpu_get_host_ticks(); > + > + tb_off = first_ppc_cpu->env.tb_env->tb_offset; > + trace_ppc_tb_adjust(tb_off, tb_off_adj, tb_off_adj - tb_off, > + (tb_off_adj - tb_off) / freq); > + > + /* Set new offset to all CPUs */ > + CPU_FOREACH(cpu) { > + PowerPCCPU *pcpu = POWERPC_CPU(cpu); > + pcpu->env.tb_env->tb_offset = tb_off_adj; > + kvmppc_set_reg_tb_offset(pcpu, pcpu->env.tb_env->tb_offset); > + } > +} > + > +static void cpu_ppc_clock_vm_state_change(void *opaque, bool running, > + RunState state) > +{ > + PPCTimebase *tb = opaque; > + > + if (running) { > + timebase_load(tb); > + } else { > + timebase_save(tb); > + } > +} > + > +/* > + * When migrating a running guest, read the clock just > + * before migration, so that the guest clock counts > + * during the events between: > + * > + * * vm_stop() > + * * > + * * pre_save() > + * > + * This reduces clock difference on migration from 5s > + * to 0.1s (when max_downtime == 5s), because sending the > + * final pages of memory (which happens between vm_stop() > + * and pre_save()) takes max_downtime. > + */ > +static int timebase_pre_save(void *opaque) > +{ > + PPCTimebase *tb = opaque; > + > + /* guest_timebase won't be overridden in case of paused guest or savevm */ > + if (!tb->runstate_paused) { > + timebase_save(tb); > + } > + > + return 0; > +} > + > static int spapr_pre_load(void *opaque) > { > int rc; > @@ -2081,6 +2176,27 @@ static const VMStateDescription vmstate_spapr_fwnmi = { > }, > }; > > +static const VMStateDescription vmstate_spapr_timebase = { > + .name = "timebase", > + .version_id = 1, > + .minimum_version_id = 1, > + .pre_save = timebase_pre_save, > + .fields = (VMStateField []) { > + VMSTATE_UINT64(guest_timebase, PPCTimebase), > + VMSTATE_INT64(time_of_the_day_ns, PPCTimebase), > + VMSTATE_END_OF_LIST() > + }, > +}; > + > +#define VMSTATE_PPC_TIMEBASE_V(_field, _state, _version) { \ > + .name = (stringify(_field)), \ > + .version_id = (_version), \ > + .size = sizeof(PPCTimebase), \ > + .vmsd = &vmstate_spapr_timebase, \ > + .flags = VMS_STRUCT, \ > + .offset = vmstate_offset_value(_state, _field, PPCTimebase), \ > +} > + > static const VMStateDescription vmstate_spapr = { > .name = "spapr", > .version_id = 3, I saw this series when it was original posted, but I failed to spot that it didn't apply to the PPC Mac machines. I have a feeling this should solve a long-running issue I've been having with decrementer migration, in which case can it be moved (or left) somewhere where this is still possible? ATB, Mark.