Re: [PATCH] xen: Don't call panic if ARM TF cpu off returns DENIED

[Dmytro Semenets <dmitry.semenets@xxxxxxxxx>]

Hi Julien,
>
> Hi Dmytro,
>
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 17/06/2022 10:10, Volodymyr Babchuk wrote:
> >>>>> Julien Grall <julien@xxxxxxx> writes:
> >>>>>
> >>>>>> On 16/06/2022 19:40, Volodymyr Babchuk wrote:
> >>>>>>> Hi Julien,
> >>>>>>
> >>>>>> Hi Volodymyr,
> >>>>>>
> >>>>>>> Julien Grall <julien@xxxxxxx> writes:
> >>>>>>>
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> On 16/06/2022 14:55, dmitry.semenets@xxxxxxxxx wrote:
> >>>>>>>>> From: Dmytro Semenets <dmytro_semenets@xxxxxxxx>
> >>>>>>>>> According to PSCI specification ARM TF can return DENIED on CPU OFF.
> >>>>>>>>
> >>>>>>>> I am confused. The spec is talking about Trusted OS and not
> >>>>>>>> firmware. The docummentation is also not specific to ARM Trusted
> >>>>>>>> Firmware. So did you mean "Trusted OS"?
> >>>>>>> It should be "firmware", I believe.
> >>>>>>
> >>>>>> Hmmm... I couldn't find a reference in the spec suggesting that
> >>>>>> CPU_OFF could return DENIED because of the firmware. Do you have a
> >>>>>> pointer to the spec?
> >>>>>
> >>>>> Ah, looks like we are talking about different things. Indeed, CPU_OFF
> >>>>> can return DENIED only because of Trusted OS. But entity that *returns*
> >>>>> the error to a caller is a firmware.
> >>>>
> >>>> Right, the interesting part is *why* DENIED is returned not *who*
> >>>> returns it.
> >>> ARM TF returns DENIED *only* for the platform I have.
> >>> We have a dissonance between spec and xen implementation because
> >>> DENIED returned by
> >>> ARM TF or Thrusted OS or whatever is not a reason for panic.
> >>
> >> I agree that's not a reason for panic. However, knowing the reason does
> >> help to figure out the correct approach.
> >>
> >> For instance, one could have suggest to migrate the trusted OS to
> >> another pCPU. But this would not have worked for you because the DENIED
> >> is not about that.
> >>
> >>> And we
> >>> have issues with this.
> >>> If machine_restart() behaviour is more or less correct  (sometimes
> >>> reports about panic but restarts the machine)
> >>
> >> Right...
> >>
> >>> but machine_halt() doesn't work at al
> >> ... this should also be the case here because machine_halt() could also
> >> be called from cpu0. So I am a bit confused why you are saying it never
> >> works.
> > If machine_halt() called on a CPU other than CPU0 it caused panic and 
> > reboot.
> > If it called on a CPU0 it also caused panic but after system power off
> > and reboot
> > is not issued. In this state you can still call the xen console. But
> > you can't reboot the system.
>
> I am lost. In a previous e-mail you said that PSCI CPU_OFF would return
> DENIED on CPU0. IOW, I understood that for other CPUs, it would succeed.
I'm sorry I confused You.
Yes it causes panic and prints it will be rebooted but actual reboot
doesn't happen.

>
> But here, you are tell me the opposite:
>
> "If it called on a CPU0 it also caused panic but after system power off
>   and reboot".
>
> If machine_halt() is called from CPU0, then CPU_OFF should not be called
> on CPU0. So where is that panic coming from?
>
> >>
> >>> Transit execution to CPU0 for my understanding is a workaround and
> >>> this approach will fix
> >>> machine_restart() function but will not fix machine_halt().
> >>
> >> I would say it is a more specific case of what the spec suggests (see
> >> below). But it should fix both machine_restart() and machine_halt()
> >> because the last CPU running will be CPU0. So Xen would call SYSTEM_*
> >> rather than CPU_OF. So I don't understand why you think it will fix one
> >> but not the other.
> > Looks like this is specific for my HW case. SYSTEM_OFF doesn't stop
> > the whole system.
>
> Hmmm... All the other CPUs should be off (or spinning with interrupt
> disabled), so are you saying that SYSTEM_OFF return?
Yes. SYSTEM_OFF returns on my HW. This is reason when CPU_OFF for CPU0 happens.
>
> Cheers,
>
> --
> Julien Grall