Re: [Qemu-devel] [PATCH v3 4/5] target/ppc: Handle NMI guest exit

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3459 bytes --]

On Mon, Aug 21, 2017 at 06:00:52PM +0530, Aravinda Prasad wrote:
> 
> 
> On Thursday 17 August 2017 07:27 AM, David Gibson wrote:
> > On Wed, Aug 16, 2017 at 02:42:39PM +0530, Aravinda Prasad wrote:
> >> Memory error such as bit flips that cannot be corrected
> >> by hardware are passed on to the kernel for handling.
> >> If the memory address in error belongs to guest then
> >> guest kernel is responsible for taking suitable action.
> >> Patch [1] enhances KVM to exit guest with exit reason
> >> set to KVM_EXIT_NMI in such cases.
> >>
> >> This patch handles KVM_EXIT_NMI exit. If the guest OS
> >> has registered the machine check handling routine by
> >> calling "ibm,nmi-register", then the handler builds
> >> the error log and invokes the registered handler else
> >> invokes the handler at 0x200.
> >>
> >> [1] https://www.spinics.net/lists/kvm-ppc/msg12637.html
> >> 	(e20bbd3d and related commits)
> >>
> >> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
> >> ---
> >>  hw/ppc/spapr.c       |    4 ++
> >>  target/ppc/kvm.c     |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  target/ppc/kvm_ppc.h |   81 +++++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 171 insertions(+)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index 0bb2c4a..6cc3f69 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -2346,6 +2346,10 @@ static void ppc_spapr_init(MachineState *machine)
> >>          error_report("Could not get size of LPAR rtas '%s'", filename);
> >>          exit(1);
> >>      }
> >> +
> >> +    /* Resize blob to accommodate error log. */
> >> +    spapr->rtas_size = RTAS_ERRLOG_OFFSET + sizeof(struct RtasMCELog);
> >> +
> >>      spapr->rtas_blob = g_malloc(spapr->rtas_size);
> >>      if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 0) {
> >>          error_report("Could not load LPAR rtas '%s'", filename);
> >> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> >> index 8571379..73f64ed 100644
> >> --- a/target/ppc/kvm.c
> >> +++ b/target/ppc/kvm.c
> >> @@ -1782,6 +1782,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> >>          ret = 0;
> >>          break;
> >>  
> >> +    case KVM_EXIT_NMI:
> >> +        DPRINTF("handle NMI exception\n");
> >> +        ret = kvm_handle_nmi(cpu);
> >> +        break;
> >> +
> >>      default:
> >>          fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
> >>          ret = -1;
> >> @@ -2704,6 +2709,87 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
> >>      return data & 0xffff;
> >>  }
> >>  
> >> +int kvm_handle_nmi(PowerPCCPU *cpu)
> > 
> > So you only handle NMIs with KVM.  Wouldn't it make sense to also
> > handle them for TCG (where they can be triggered with the "nmi"
> > command on the monitor).
> 
> For TCG it is already handled in spapr_nmi() which ultimately branches
> to guest's 0x200 vector. Even with KVM when KVM_CAP_PPC_FWNMI is not
> enabled we branch to guest's 0x200 vector. Should TCG behave as if
> KVM_CAP_PPC_FWNMI is not enabled?

No, we should implement the FWNMI feature for TCG, which means it
needs to respect the address given by nmi-register.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]