Re: [PATCH v3 01/27] target: Set CPUClass::vmsd instead of DeviceClass::vmsd - Philippe Mathieu-Daudé

From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: Eduardo Habkost <ehabkost@redhat.com>,
	Juan Quintela <quintela@redhat.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Sarah Harris" <S.E.Harris@kent.ac.uk>,
	"Cornelia Huck" <cohuck@redhat.com>,
	"Sagar Karandikar" <sagark@eecs.berkeley.edu>,
	"David Hildenbrand" <david@redhat.com>,
	"Anthony Green" <green@moxielogic.com>,
	"Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>,
	qemu-devel@nongnu.org, "Max Filippov" <jcmvbkbc@gmail.com>,
	"Taylor Simpson" <tsimpson@quicinc.com>,
	"Alistair Francis" <Alistair.Francis@wdc.com>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	"Guan Xuetao" <gxt@mprc.pku.edu.cn>,
	"Marek Vasut" <marex@denx.de>,
	"Yoshinori Sato" <ysato@users.sourceforge.jp>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Claudio Fontana" <cfontana@suse.de>,
	qemu-ppc@nongnu.org, "Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Artyom Tarasenko" <atar4qemu@gmail.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Greg Kurz" <groug@kaod.org>,
	qemu-s390x@nongnu.org, qemu-arm@nongnu.org,
	"Michael Rolnik" <mrolnik@gmail.com>,
	"Stafford Horne" <shorne@gmail.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"David Gibson" <david@gibson.dropbear.id.au>,
	qemu-riscv@nongnu.org,
	"Bastian Koppelmann" <kbastian@mail.uni-paderborn.de>,
	"Chris Wulff" <crwulff@gmail.com>,
	"Laurent Vivier" <laurent@vivier.eu>,
	"Michael Walle" <michael@walle.cc>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Aleksandar Rikalo" <aleksandar.rikalo@syrmia.com>,
	"Aurelien Jarno" <aurelien@aurel32.net>
Subject: Re: [PATCH v3 01/27] target: Set CPUClass::vmsd instead of DeviceClass::vmsd
Date: Thu, 22 Apr 2021 11:55:27 +0200	[thread overview]
Message-ID: <cccb8e5c-55cb-3f46-1391-ca96ab7c27ce@amsat.org> (raw)
In-Reply-To: <20210421220333.bkxo6zriqe6w3rim@habkost.net>

+Juan

On 4/22/21 12:03 AM, Eduardo Habkost wrote:
> On Tue, Mar 02, 2021 at 03:57:52PM +0100, Philippe Mathieu-Daudé wrote:
>> The cpu model is the single device available in user-mode.
>> Since we want to restrict some fields to user-mode emulation,
>> we prefer to set the vmsd field of CPUClass, rather than the
>> DeviceClass one.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Is this going to have an externally visible effect?

On system emulation, no, because unchanged.

On user emulation migration it is not used, the symbol is here
to satisfy linking:

include/hw/core/cpu.h-1070-#ifdef CONFIG_SOFTMMU
include/hw/core/cpu.h-1071-extern const VMStateDescription
vmstate_cpu_common;
include/hw/core/cpu.h-1072-#else
include/hw/core/cpu.h:1073:#define vmstate_cpu_common vmstate_dummy
include/hw/core/cpu.h-1074-#endif
include/migration/vmstate.h:197:extern const VMStateDescription
vmstate_dummy;
stubs/vmstate.c:4:const VMStateDescription vmstate_dummy = {};

> If it does, how can we make sure it's safe?
> 
> If it does not, do you know why CPUClass::vmsd exists in the
> first place?

My guess is CPUState is the only device used in user emulation,
so it would be a way to restrict the vmstate_dummy to CPU and
not to any DeviceState?

But looking at the introductory commit:

commit b170fce3dd06372f7bfec9a780ebcb1fce6d57e4
Author: Andreas Färber <afaerber@suse.de>
Date:   Sun Jan 20 20:23:22 2013 +0100

    cpu: Register VMStateDescription through CPUState

    In comparison to DeviceClass::vmsd, CPU VMState is split in two,
    "cpu_common" and "cpu", and uses cpu_index as instance_id instead of -1.
    Therefore add a CPU-specific CPUClass::vmsd field.

    Unlike the legacy CPUArchState registration, rather register CPUState.

Juan, do you remember?

> 
> Do you think it would be simpler to just squash this patch into
>   [PATCH v3 08/27] cpu: Move CPUClass::vmsd to SysemuCPUOps
> ?

Certainly cleaner, I'll respin!

Thanks,

Phil.