[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH] spapr/xive: Sanity checks of OV5 during CAS
From: |
Greg Kurz |
Subject: |
Re: [Qemu-ppc] [PATCH] spapr/xive: Sanity checks of OV5 during CAS |
Date: |
Thu, 16 May 2019 08:58:14 +0200 |
On Thu, 16 May 2019 12:09:57 +0530
Satheesh Rajendran <address@hidden> 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 <address@hidden>
> > Signed-off-by: Greg Kurz <address@hidden>
> > ---
> > 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)");
>
> Looks like there is a typo in the error msg reported, instead it should be
> something like below
>
> "Guest requested unavailable interrupt mode(XIVE), please use supported
> interrupt mode(ic-type=xics)"
>
Hmm... Agreed that it may be worth adding some hint for the user to proceed
but...
> Same for the previous check aswell.
>
> Coz, I booted 3.10(kernel) guest with ic-type=xive and got a below error,
> which seems wrong
>
> 2019-05-16T06:24:58.713261Z qemu-system-ppc64: Guest requested unavailable
> interrupt mode (XICS)
>
... this message is ok: the 3.10 kernel based guest _does_ ask for XICS,
which isn't available because of ic-mode=xive.
Cheers,
--
Greg
> Regards,
> -Satheesh.
>
> > + exit(EXIT_FAILURE);
> > + }
> > + }
> > +
> > /*
> > * Generate a machine reset when we have an update of the
> > * interrupt mode. Only required when the machine supports both
> >
>