Re: [PATCH v1 ppc-for-5.0 2/2] ppc/spapr: Support reboot of secure pseries guest

[Bharata B Rao]

On Tue, Dec 10, 2019 at 04:05:36PM +1100, David Gibson wrote:
> On Tue, Dec 10, 2019 at 03:03:01PM +1100, Alexey Kardashevskiy wrote:
> > 
> > 
> > On 10/12/2019 14:50, Bharata B Rao wrote:
> > > On Tue, Dec 10, 2019 at 02:28:51PM +1100, David Gibson wrote:
> > >> On Mon, Dec 09, 2019 at 12:30:12PM +0530, Bharata B Rao wrote:
> > >>> A pseries guest can be run as a secure guest on Ultravisor-enabled
> > >>> POWER platforms. When such a secure guest is reset, we need to
> > >>> release/reset a few resources both on ultravisor and hypervisor side.
> > >>> This is achieved by invoking this new ioctl KVM_PPC_SVM_OFF from the
> > >>> machine reset path.
> > >>>
> > >>> As part of this ioctl, the secure guest is essentially transitioned
> > >>> back to normal mode so that it can reboot like a regular guest and
> > >>> become secure again.
> > >>>
> > >>> This ioctl has no effect when invoked for a normal guest.
> > >>>
> > >>> Signed-off-by: Bharata B Rao <address@hidden>
> > >>> ---
> > >>>  hw/ppc/spapr.c       | 1 +
> > >>>  target/ppc/kvm.c     | 7 +++++++
> > >>>  target/ppc/kvm_ppc.h | 6 ++++++
> > >>>  3 files changed, 14 insertions(+)
> > >>>
> > >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > >>> index f11422fc41..4c7ad3400d 100644
> > >>> --- a/hw/ppc/spapr.c
> > >>> +++ b/hw/ppc/spapr.c
> > >>> @@ -1597,6 +1597,7 @@ static void spapr_machine_reset(MachineState 
> > >>> *machine)
> > >>>      void *fdt;
> > >>>      int rc;
> > >>>  
> > >>> +    kvmppc_svm_off();
> > >>
> > >> If you're going to have this return an error value, you should really
> > >> check it here.
> > > 
> > > I could, by spapr_machine_reset() and the callers don't propagate the
> > > errors up. So may be I could print a warning instead when ioctl fails?
> > 
> > An error here means you cannot restart the machine and should probably
> > suspend, or try until it is not EBUSY (==all threads have stopped?).
> 
> Right, if this fails, something has gone badly wrong.  You should
> absolutely print a message, and in fact it might be appropriate to
> quit outright.  IIUC the way PEF resets work, a failure here means you
> won't be able to boot after the reset, since the guest memory will
> still be inaccessible to the host.

Correct. I will send next version with a message and abort() added in
the ioctl failure path.

Regards,
Bharata.