[Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS

[Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS

[Greg Kurz]

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 <sathnaga@linux.vnet.ibm.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 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

Re: [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS

[David Gibson]

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

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 <sathnaga@linux.vnet.ibm.com>
> Signed-off-by: Greg Kurz <groug@kaod.org>

Seems sensible to me, applied.

> ---
>  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
> 

-- 
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 --]

Re: [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS

[Cédric Le Goater]

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 <sathnaga@linux.vnet.ibm.com>
>> Signed-off-by: Greg Kurz <groug@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
>>
> 

Re: [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS

[Satheesh Rajendran]

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 <sathnaga@linux.vnet.ibm.com>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  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)"

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)

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
> 

Re: [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS

[Greg Kurz]

On Thu, 16 May 2019 12:09:57 +0530
Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> 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 <sathnaga@linux.vnet.ibm.com>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >  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
> >   
> 

Re: [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS

[David Gibson]

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

On Thu, May 16, 2019 at 08:58:14AM +0200, Greg Kurz wrote:
> On Thu, 16 May 2019 12:09:57 +0530
> Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> 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 <sathnaga@linux.vnet.ibm.com>
> > > Signed-off-by: Greg Kurz <groug@kaod.org>
> > > ---
> > >  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.

I won't be sending a pull request for a little while yet, so if you
want to send a followup improving the messages I can fold it into the
original patch in my tree.

-- 
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 --]

Re: [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS

[Greg Kurz]

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

On Thu, 16 May 2019 21:46:01 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:
[...]
> 
> I won't be sending a pull request for a little while yet, so if you
> want to send a followup improving the messages I can fold it into the
> original patch in my tree.
> 

Done.

https://patchwork.ozlabs.org/patch/1100396/


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