Re: [PATCH qemu] spapr: Force 32bit when resetting a core

["Cédric Le Goater" <clg@kaod.org>]

On 1/19/22 12:05, Cédric Le Goater wrote:
> On 1/16/22 17:45, Peter Maydell wrote:
>> On Fri, 7 Jan 2022 at 07:29, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>
>>> "PowerPC Processor binding to IEEE 1275" says in
>>> "8.2.1. Initial Register Values" that the initial state is defined as
>>> 32bit so do it for both SLOF and VOF.
>>>
>>> This should not cause behavioral change as SLOF switches to 64bit very
>>> early anyway. As nothing enforces LE anywhere, this drops it for VOF.
>>>
>>> The goal is to make VOF work with TCG as otherwise it barfs with
>>> qemu: fatal: TCG hflags mismatch (current:0x6c000004 rebuilt:0x6c000000)
>>
>> If you get this assert it means that something is changing
>> the CPU state and not calling the function to recalculate
>> the hflags (which are basically caching part of the CPU state).
>> So I don't know whether this patch is correct or not in updating
>> MSR bits, but in any case I think it is only masking the
>> missing-hflags-update that is causing the assertion...
> 
> 
> yes. Currently, spapr_vof_reset() is called from the pseries
> machine reset handler and modifies brutally the MSR without
> calling hreg_compute_hflags()to update the hflags. Hence
> the warning.
> 
> The proposal moves the MSR update under the pseries CPU reset
> handler: spapr_reset_vcpu() where it should be. spapr_reset_vcpu()
> also updates the LPCR register and calls hreg_compute_hflags()
> when done.
> 
> The question we all had was why it was set to 64bit initially
> by commit 8b9f2118ca40 which seems to be in contradiction with
> the PAPR specs saying the CPUs should start in 32bit mode.
> It is not clear but I didn't see any regression on pseries or
> on the macbook machine using 970 CPUs. I think we are fine.

With that said,

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.