On 5/16/19 3:24 AM, David Gibson wrote: > On Wed, May 15, 2019 at 07:04:24PM +0200, Greg Kurz wrote: >> If a machine is started with ic-mode=xive but the guest only knows >> about XICS, eg. an RHEL 7.6 guest, the kernel panics. This is >> expected but a bit unfortunate since the crash doesn't provide >> much information for the end user to guess what's happening. >> >> Detect that during CAS and exit QEMU with a proper error message >> instead, like it is already done for the MMU. >> >> Even if this is less likely to happen, the opposite case of a guest >> that only knows about XIVE would certainly fail all the same if the >> machine is started with ic-mode=xics. >> >> Also, the only valid values a guest can pass in byte 23 of OV5 during >> CAS are 0b00 (XIVE legacy mode) and 0b01 (XIVE exploitation mode). Any >> other value is a bug, at least with the current spec. Again, it does >> not seem right to let the guest go on without a precise idea of the >> interrupt mode it asked for. >> >> Handle these cases as well. >> >> Reported-by: Satheesh Rajendran <sathn...@linux.vnet.ibm.com> >> Signed-off-by: Greg Kurz <gr...@kaod.org> > > Seems sensible to me, applied.
yes. I would have splitted the patch though. But this is minor. Thanks, C. >> --- >> hw/ppc/spapr_hcall.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c >> index 6c16d2b12040..63a55614b83d 100644 >> --- a/hw/ppc/spapr_hcall.c >> +++ b/hw/ppc/spapr_hcall.c >> @@ -1513,6 +1513,7 @@ static target_ulong >> h_client_architecture_support(PowerPCCPU *cpu, >> bool guest_radix; >> Error *local_err = NULL; >> bool raw_mode_supported = false; >> + bool guest_xive; >> >> cas_pvr = cas_check_pvr(spapr, cpu, &addr, &raw_mode_supported, >> &local_err); >> if (local_err) { >> @@ -1545,10 +1546,17 @@ static target_ulong >> h_client_architecture_support(PowerPCCPU *cpu, >> error_report("guest requested hash and radix MMU, which is >> invalid."); >> exit(EXIT_FAILURE); >> } >> + if (spapr_ovec_test(ov5_guest, OV5_XIVE_BOTH)) { >> + error_report("guest requested an invalid interrupt mode"); >> + exit(EXIT_FAILURE); >> + } >> + >> /* The radix/hash bit in byte 24 requires special handling: */ >> guest_radix = spapr_ovec_test(ov5_guest, OV5_MMU_RADIX_300); >> spapr_ovec_clear(ov5_guest, OV5_MMU_RADIX_300); >> >> + guest_xive = spapr_ovec_test(ov5_guest, OV5_XIVE_EXPLOIT); >> + >> /* >> * HPT resizing is a bit of a special case, because when enabled >> * we assume an HPT guest will support it until it says it >> @@ -1632,6 +1640,22 @@ static target_ulong >> h_client_architecture_support(PowerPCCPU *cpu, >> ov5_updates) != 0); >> } >> >> + /* >> + * Ensure the guest asks for an interrupt mode we support; otherwise >> + * terminate the boot. >> + */ >> + if (guest_xive) { >> + if (spapr->irq->ov5 == SPAPR_OV5_XIVE_LEGACY) { >> + error_report("Guest requested unavailable interrupt mode >> (XIVE)"); >> + exit(EXIT_FAILURE); >> + } >> + } else { >> + if (spapr->irq->ov5 == SPAPR_OV5_XIVE_EXPLOIT) { >> + error_report("Guest requested unavailable interrupt mode >> (XICS)"); >> + exit(EXIT_FAILURE); >> + } >> + } >> + >> /* >> * Generate a machine reset when we have an update of the >> * interrupt mode. Only required when the machine supports both >> >