* [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS
@ 2019-05-15 17:04 Greg Kurz
2019-05-16 1:24 ` David Gibson
2019-05-16 6:39 ` Satheesh Rajendran
0 siblings, 2 replies; 7+ messages in thread
From: Greg Kurz @ 2019-05-15 17:04 UTC (permalink / raw)
To: David Gibson, Cédric Le Goater, Greg Kurz
Cc: Satheesh Rajendran, qemu-ppc, qemu-devel
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS
2019-05-15 17:04 [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS Greg Kurz
@ 2019-05-16 1:24 ` David Gibson
2019-05-16 6:25 ` Cédric Le Goater
2019-05-16 6:39 ` Satheesh Rajendran
1 sibling, 1 reply; 7+ messages in thread
From: David Gibson @ 2019-05-16 1:24 UTC (permalink / raw)
To: Greg Kurz; +Cc: Satheesh Rajendran, qemu-ppc, Cédric Le Goater, qemu-devel
[-- 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 --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS
2019-05-16 1:24 ` David Gibson
@ 2019-05-16 6:25 ` Cédric Le Goater
0 siblings, 0 replies; 7+ messages in thread
From: Cédric Le Goater @ 2019-05-16 6:25 UTC (permalink / raw)
To: David Gibson, Greg Kurz; +Cc: Satheesh Rajendran, qemu-ppc, qemu-devel
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
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS
2019-05-15 17:04 [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS Greg Kurz
2019-05-16 1:24 ` David Gibson
@ 2019-05-16 6:39 ` Satheesh Rajendran
2019-05-16 6:58 ` Greg Kurz
1 sibling, 1 reply; 7+ messages in thread
From: Satheesh Rajendran @ 2019-05-16 6:39 UTC (permalink / raw)
To: Greg Kurz
Cc: qemu-devel, Satheesh Rajendran, qemu-ppc, Cédric Le Goater,
David Gibson
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS
2019-05-16 6:39 ` Satheesh Rajendran
@ 2019-05-16 6:58 ` Greg Kurz
2019-05-16 11:46 ` David Gibson
0 siblings, 1 reply; 7+ messages in thread
From: Greg Kurz @ 2019-05-16 6:58 UTC (permalink / raw)
To: Satheesh Rajendran
Cc: qemu-devel, qemu-ppc, Cédric Le Goater, David Gibson
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
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS
2019-05-16 6:58 ` Greg Kurz
@ 2019-05-16 11:46 ` David Gibson
2019-05-16 12:27 ` Greg Kurz
0 siblings, 1 reply; 7+ messages in thread
From: David Gibson @ 2019-05-16 11:46 UTC (permalink / raw)
To: Greg Kurz; +Cc: qemu-ppc, Satheesh Rajendran, Cédric Le Goater, qemu-devel
[-- 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 --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS
2019-05-16 11:46 ` David Gibson
@ 2019-05-16 12:27 ` Greg Kurz
0 siblings, 0 replies; 7+ messages in thread
From: Greg Kurz @ 2019-05-16 12:27 UTC (permalink / raw)
To: David Gibson
Cc: qemu-ppc, Satheesh Rajendran, Cédric Le Goater, qemu-devel
[-- 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 --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-05-16 12:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-15 17:04 [Qemu-devel] [PATCH] spapr/xive: Sanity checks of OV5 during CAS Greg Kurz
2019-05-16 1:24 ` David Gibson
2019-05-16 6:25 ` Cédric Le Goater
2019-05-16 6:39 ` Satheesh Rajendran
2019-05-16 6:58 ` Greg Kurz
2019-05-16 11:46 ` David Gibson
2019-05-16 12:27 ` Greg Kurz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.