Re: [PATCH] cpu: Add starts_halted() method

[Greg Kurz]

On Thu, 9 Jul 2020 12:18:06 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> On 7/9/20 11:54 AM, Greg Kurz wrote:
> > On Thu, 9 Jul 2020 07:11:09 +0200
> > Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >> On 7/8/20 11:39 PM, Eduardo Habkost wrote:
> >>> On Wed, Jul 08, 2020 at 06:45:57PM +0200, Philippe Mathieu-Daudé wrote:
> >>>> On 7/8/20 5:25 PM, Eduardo Habkost wrote:
> >>>>> On Wed, Jul 08, 2020 at 02:14:03PM +0100, Peter Maydell wrote:
> >>>>>> On Wed, 8 Jul 2020 at 12:12, David Gibson 
> >>>>>> <david@gibson.dropbear.id.au> wrote:
> >>>>>>> On Wed, Jul 08, 2020 at 10:38:29AM +0200, Philippe Mathieu-Daudé 
> >>>>>>> wrote:
> >>>>>>>> Class boolean field certainly sounds better, but I am not sure this
> >>>>>>>> is a property of the machine. Rather the arch? So move the field
> >>>>>>>> to CPUClass? Maybe not, let's discuss :)
> >>>>>>>
> >>>>>>> It is absolutely a property of the machine.  e.g. I don't think we
> >>>>>>> want this for powernv.  pseries is a bit of a special case since it is
> >>>>>>> explicitly a paravirt platform.  But even for emulated hardware, the
> >>>>>>> board can absolutely strap things so that cpus do or don't start
> >>>>>>> immediately.
> >>>>>>
> >>>>>> It's a property of the individual CPU, I think. One common setup
> >>>>>> for Arm systems is that the primary CPU starts powered up but
> >>>>>> the secondaries all start powered down.
> >>>>>
> >>>>> Both statements can be true.  It can be a property of the
> >>>>> individual CPU (although I'm not convinced it has to), but it
> >>>>> still needs to be controlled by the machine.
> >>>>
> >>>> From what said Peter, I understand this is a property of the
> >>>> chipset. Chipsets are modelled unevenly.
> >>>>
> >>>> IIUC QEMU started with single-core CPUs.
> >>>> CPUState had same meaning for 'core' or 'cpu', 1-1 mapping.
> >>>>
> >>>> Then multicore CPUs could be easily modelled using multiple
> >>>> single-core CPUs, usually created in the machine code.
> >>>>
> >>>> Then we moved to SoC models, creating the cores in the SoC.
> >>>> Some SoCs have array of cores, eventually heterogeneous
> >>>> (see the ZynqMP). We have containers of CPUState.
> >>>>
> >>>> On an ARM-based SoC, you might have the first core started
> >>>> (as said Peter) or not.
> >>>>
> >>>> BCM2836 / BCM2837 and ZynqMP start will all ARM cores off.
> >>>> On the BCM chipsets, a DSP core will boot the ARM cores.
> >>>> On the ZynqMP, a MicroBlaze core boots them.
> >>>> As QEMU doesn't models heterogeneous architectures, we start
> >>>> modelling after the unmodelled cores booted us, so either one
> >>>> or all cores on.
> >>>>
> >>>> In this case, we narrowed down the 'start-powered-off' field
> >>>> to the SoC, which happens to be how ARM SoCs are modelled.
> >>>
> >>> I was not aware of the start-powered-off property.  If we make it
> >>> generic, we can just let spapr use it.
> >>>
> >>>>
> >>>>
> >>>> Chipsets providing a JTAG interface can have a SRST signal,
> >>>> the "system reset". When a JTAG probe is attached, it can
> >>>> keeps the whole chipset in a reset state. This is equivalent
> >>>> to QEMU '-S' mode (single step mode).
> >>>>
> >>>>
> >>>> I don't know about pseries hardware, but if this is 'explicit
> >>>> to paravirt platform', then I expect this to be the same with
> >>>> other accelerators/architectures.
> >>>>
> >>>> If paravirtualized -> cores start off by default. Let the
> >>>> hypervisor start them. So still a property of the CPUState
> >>>> depending on the accelerator used?
> >>>
> >>> I don't understand this part.  Why would this depend on the
> >>> accelerator?
> >>
> >> Because starting a virtualized machine with all cores powered-off
> >> with TCG accelerator should at least emit a warning? Or change
> >> the behavior and start them powered-on? This is machine-specific
> >> although.
> >>
> > 
> > FYI, PAPR requires all vCPUs to be "stopped" by default. It is up to the
> > guest to start them explicitly through an RTAS call. The hypervisor is
> > only responsible to start a single vCPU (see spapr_cpu_set_entry_state()
> > called from spapr_machine_reset()) to be able to boot the guest.
> > 
> > So I'm not sure to see how that would depend on the accelerator...
> 
> $ qemu-system-ppc64 -M pseries-5.0,accel=tcg -d in_asm
> qemu-system-ppc64: warning: TCG doesn't support requested feature,
> cap-cfpc=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature,
> cap-sbbc=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature,
> cap-ibs=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature,
> cap-ccf-assist=on
> ----------------
> IN:
> 0x00000100:  48003f00  b        0x4000
> 
> ----------------
> IN:
> 0x00004000:  7c7f1b78  mr       r31, r3
> 0x00004004:  7d6000a6  mfmsr    r11
> 0x00004008:  3980a000  li       r12, 0xa000
> 0x0000400c:  798c83c6  sldi     r12, r12, 0x30
> 0x00004010:  7d6b6378  or       r11, r11, r12
> 0x00004014:  7d600164  mtmsrd   r11
> ...
> 
> The vCPU doesn't seem stopped to me...
> 
> Am I missing something?
> 

Yeah this is the boot vCPU which is required to be started
by the platform as explained above, but if you had more
vCPUs the other ones would be stopped until the guest OS
asks us to start them.