diff mbox series

spapr: nested: Add support for DPDES SPR in GSB for TCG L0

Message ID 20241017110033.3929988-1-amachhiw@linux.ibm.com
State New
Headers show
Series spapr: nested: Add support for DPDES SPR in GSB for TCG L0 | expand

Commit Message

Amit Machhiwal Oct. 17, 2024, 11 a.m. UTC 
The DPDES support for doorbell emulation and handling for KVM on PAPR
guests was added in Linux via [1]. Subsequently, a new GSB element for
DPDES was added in Linux; the same has been missing in QEMU L0. Add
support for DPDES register's APIv2 GSB element and required handling in
`spapr_nested.c`.

Currently, booting a KVM guest inside a QEMU TCG guest fails with the
below crash. The crash is encountered when GUEST_RUN_VCPU hcall made
into QEMU TCG L0 fails because H_INVALID_ELEMENT_VALUE is returned as
the mapping of the element ID corresponding to DPDES (unknown to QEMU
TCG L0) in GSR (Guest State Request) of TCG guest's KVM to the GSB
(Guest State Buffer) elements of QEMU TCG L0 fails.

 KVM: unknown exit, hardware reason ffffffffffffffea
 NIP 0000000000000100   LR 0000000000000000 CTR 0000000000000000 XER 0000000000000000 CPU#0
 MSR 0000000000003000 HID0 0000000000000000  HF 6c002000 iidx 3 didx 3
 TB 00000000 00000000 DECR 0
 GPR00 0000000000000000 0000000000000000 0000000000000000 000000007fe00000
 GPR04 0000000000000000 0000000000000000 0000000000000000 0000000000000000
 GPR08 0000000000000000 0000000000000000 0000000000000000 0000000000000000
 GPR12 0000000000000000 0000000000000000 0000000000000000 0000000000000000
 GPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000
 GPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000
 GPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000
 GPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000
 CR 00000000  [ -  -  -  -  -  -  -  -  ]     RES 000@ffffffffffffffff
  SRR0 0000000000000000  SRR1 0000000000000000    PVR 0000000000801200 VRSAVE 0000000000000000
 SPRG0 0000000000000000 SPRG1 0000000000000000  SPRG2 0000000000000000  SPRG3 0000000000000000
 SPRG4 0000000000000000 SPRG5 0000000000000000  SPRG6 0000000000000000  SPRG7 0000000000000000
 HSRR0 0000000000000000 HSRR1 0000000000000000
  CFAR 0000000000000000
  LPCR 0000000000560413
  PTCR 0000000000000000   DAR 0000000000000000  DSISR 0000000000000000

Fix this by adding the required support and handling in QEMU TCG L0.

[1] https://lore.kernel.org/all/20240605113913.83715-1-gautam@linux.ibm.com/

Fixes: 4a575f9a0567 ("spapr: nested: Initialize the GSB elements lookup table.")
Suggested-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Reviewed-by: Vaibhav Jain <vaibhav@linux.ibm.com>
Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
---
 hw/ppc/spapr_nested.c         | 3 +++
 include/hw/ppc/spapr_nested.h | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)


base-commit: aa54f5be44be786636a5d51cc1612ad208a24849

Comments

Harsh Prateek Bora Oct. 18, 2024, 5:17 a.m. UTC | #1 
Hi Amit,

On 10/17/24 16:30, Amit Machhiwal wrote:
> The DPDES support for doorbell emulation and handling for KVM on PAPR
> guests was added in Linux via [1]. Subsequently, a new GSB element for
> DPDES was added in Linux; the same has been missing in QEMU L0. Add

s/QEMU L0/ TCG L0 implementation?

> support for DPDES register's APIv2 GSB element and required handling in
> `spapr_nested.c`.
> 
> Currently, booting a KVM guest inside a QEMU TCG guest fails with the
> below crash. The crash is encountered when GUEST_RUN_VCPU hcall made
> into QEMU TCG L0 fails because H_INVALID_ELEMENT_VALUE is returned as
> the mapping of the element ID corresponding to DPDES (unknown to QEMU
> TCG L0) in GSR (Guest State Request) of TCG guest's KVM to the GSB
> (Guest State Buffer) elements of QEMU TCG L0 fails.

GSB full form would be more appropriate along with first mention of GSB 
in above text.

With that:
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

> 
>   KVM: unknown exit, hardware reason ffffffffffffffea
>   NIP 0000000000000100   LR 0000000000000000 CTR 0000000000000000 XER 0000000000000000 CPU#0
>   MSR 0000000000003000 HID0 0000000000000000  HF 6c002000 iidx 3 didx 3
>   TB 00000000 00000000 DECR 0
>   GPR00 0000000000000000 0000000000000000 0000000000000000 000000007fe00000
>   GPR04 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   GPR08 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   GPR12 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   GPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   GPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   GPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   GPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   CR 00000000  [ -  -  -  -  -  -  -  -  ]     RES 000@ffffffffffffffff
>    SRR0 0000000000000000  SRR1 0000000000000000    PVR 0000000000801200 VRSAVE 0000000000000000
>   SPRG0 0000000000000000 SPRG1 0000000000000000  SPRG2 0000000000000000  SPRG3 0000000000000000
>   SPRG4 0000000000000000 SPRG5 0000000000000000  SPRG6 0000000000000000  SPRG7 0000000000000000
>   HSRR0 0000000000000000 HSRR1 0000000000000000
>    CFAR 0000000000000000
>    LPCR 0000000000560413
>    PTCR 0000000000000000   DAR 0000000000000000  DSISR 0000000000000000
> 
> Fix this by adding the required support and handling in QEMU TCG L0.
> 
> [1] https://lore.kernel.org/all/20240605113913.83715-1-gautam@linux.ibm.com/
> 
> Fixes: 4a575f9a0567 ("spapr: nested: Initialize the GSB elements lookup table.")
> Suggested-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Reviewed-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com > ---
>   hw/ppc/spapr_nested.c         | 3 +++
>   include/hw/ppc/spapr_nested.h | 3 ++-
>   2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
> index c02785756c1e..b696ad537a77 100644
> --- a/hw/ppc/spapr_nested.c
> +++ b/hw/ppc/spapr_nested.c
> @@ -194,6 +194,7 @@ static void nested_save_state(struct nested_ppc_state *save, PowerPCCPU *cpu)
>           save->fscr = env->spr[SPR_FSCR];
>           save->pspb = env->spr[SPR_PSPB];
>           save->ctrl = env->spr[SPR_CTRL];
> +        save->dpdes = env->spr[SPR_DPDES];
>           save->vrsave = env->spr[SPR_VRSAVE];
>           save->dar = env->spr[SPR_DAR];
>           save->dsisr = env->spr[SPR_DSISR];
> @@ -293,6 +294,7 @@ static void nested_load_state(PowerPCCPU *cpu, struct nested_ppc_state *load)
>           env->spr[SPR_FSCR] = load->fscr;
>           env->spr[SPR_PSPB] = load->pspb;
>           env->spr[SPR_CTRL] = load->ctrl;
> +        env->spr[SPR_DPDES] = load->dpdes;
>           env->spr[SPR_VRSAVE] = load->vrsave;
>           env->spr[SPR_DAR] = load->dar;
>           env->spr[SPR_DSISR] = load->dsisr;
> @@ -982,6 +984,7 @@ struct guest_state_element_type guest_state_element_types[] = {
>       GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_FSCR,  fscr),
>       GUEST_STATE_ELEMENT_ENV_W(GSB_VCPU_SPR_PSPB,   pspb),
>       GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_CTRL,  ctrl),
> +    GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_DPDES, dpdes),
>       GUEST_STATE_ELEMENT_ENV_W(GSB_VCPU_SPR_VRSAVE, vrsave),
>       GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_DAR,   dar),
>       GUEST_STATE_ELEMENT_ENV_W(GSB_VCPU_SPR_DSISR,  dsisr),
> diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
> index 93ef14adcc5e..3b5cd993c256 100644
> --- a/include/hw/ppc/spapr_nested.h
> +++ b/include/hw/ppc/spapr_nested.h
> @@ -99,7 +99,8 @@
>   #define GSB_VCPU_SPR_HASHKEYR   0x1050
>   #define GSB_VCPU_SPR_HASHPKEYR  0x1051
>   #define GSB_VCPU_SPR_CTRL       0x1052
> -                    /* RESERVED 0x1053 - 0x1FFF */
> +#define GSB_VCPU_SPR_DPDES      0x1053
> +                    /* RESERVED 0x1054 - 0x1FFF */
>   #define GSB_VCPU_SPR_CR         0x2000
>   #define GSB_VCPU_SPR_PIDR       0x2001
>   #define GSB_VCPU_SPR_DSISR      0x2002
> 
> base-commit: aa54f5be44be786636a5d51cc1612ad208a24849
Harsh Prateek Bora Oct. 18, 2024, 8:43 a.m. UTC | #2 
Hi Amit,

On 10/18/24 10:47, Harsh Prateek Bora wrote:
> Hi Amit,
> 
> On 10/17/24 16:30, Amit Machhiwal wrote:
>> The DPDES support for doorbell emulation and handling for KVM on PAPR
>> guests was added in Linux via [1]. Subsequently, a new GSB element for
>> DPDES was added in Linux; the same has been missing in QEMU L0. Add
> 
> s/QEMU L0/ TCG L0 implementation?
> 
>> support for DPDES register's APIv2 GSB element and required handling in
>> `spapr_nested.c`.
>>
>> Currently, booting a KVM guest inside a QEMU TCG guest fails with the
>> below crash. The crash is encountered when GUEST_RUN_VCPU hcall made
>> into QEMU TCG L0 fails because H_INVALID_ELEMENT_VALUE is returned as
>> the mapping of the element ID corresponding to DPDES (unknown to QEMU
>> TCG L0) in GSR (Guest State Request) of TCG guest's KVM to the GSB
>> (Guest State Buffer) elements of QEMU TCG L0 fails.
> 
> GSB full form would be more appropriate along with first mention of GSB 
> in above text.
> 
> With that:
> Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> 
>>

<snip>

>> diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
>> index c02785756c1e..b696ad537a77 100644
>> --- a/hw/ppc/spapr_nested.c
>> +++ b/hw/ppc/spapr_nested.c
>> @@ -194,6 +194,7 @@ static void nested_save_state(struct 
>> nested_ppc_state *save, PowerPCCPU *cpu)
>>           save->fscr = env->spr[SPR_FSCR];
>>           save->pspb = env->spr[SPR_PSPB];
>>           save->ctrl = env->spr[SPR_CTRL];
>> +        save->dpdes = env->spr[SPR_DPDES];
>>           save->vrsave = env->spr[SPR_VRSAVE];
>>           save->dar = env->spr[SPR_DAR];
>>           save->dsisr = env->spr[SPR_DSISR];
>> @@ -293,6 +294,7 @@ static void nested_load_state(PowerPCCPU *cpu, 
>> struct nested_ppc_state *load)
>>           env->spr[SPR_FSCR] = load->fscr;
>>           env->spr[SPR_PSPB] = load->pspb;
>>           env->spr[SPR_CTRL] = load->ctrl;
>> +        env->spr[SPR_DPDES] = load->dpdes;
>>           env->spr[SPR_VRSAVE] = load->vrsave;
>>           env->spr[SPR_DAR] = load->dar;
>>           env->spr[SPR_DSISR] = load->dsisr;

Looks like we overlooked above inits already present in existing code.
I just checked above inits in nested_{save,load}_state are already
present as common code between apiv1 & apiv2. So, we only need the inits
below for initializing the lookup table.

regards,
Harsh
>> @@ -982,6 +984,7 @@ struct guest_state_element_type 
>> guest_state_element_types[] = {
>>       GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_FSCR,  fscr),
>>       GUEST_STATE_ELEMENT_ENV_W(GSB_VCPU_SPR_PSPB,   pspb),
>>       GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_CTRL,  ctrl),
>> +    GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_DPDES, dpdes),
>>       GUEST_STATE_ELEMENT_ENV_W(GSB_VCPU_SPR_VRSAVE, vrsave),
>>       GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_DAR,   dar),
>>       GUEST_STATE_ELEMENT_ENV_W(GSB_VCPU_SPR_DSISR,  dsisr),
>> diff --git a/include/hw/ppc/spapr_nested.h 
>> b/include/hw/ppc/spapr_nested.h
>> index 93ef14adcc5e..3b5cd993c256 100644
>> --- a/include/hw/ppc/spapr_nested.h
>> +++ b/include/hw/ppc/spapr_nested.h
>> @@ -99,7 +99,8 @@
>>   #define GSB_VCPU_SPR_HASHKEYR   0x1050
>>   #define GSB_VCPU_SPR_HASHPKEYR  0x1051
>>   #define GSB_VCPU_SPR_CTRL       0x1052
>> -                    /* RESERVED 0x1053 - 0x1FFF */
>> +#define GSB_VCPU_SPR_DPDES      0x1053
>> +                    /* RESERVED 0x1054 - 0x1FFF */
>>   #define GSB_VCPU_SPR_CR         0x2000
>>   #define GSB_VCPU_SPR_PIDR       0x2001
>>   #define GSB_VCPU_SPR_DSISR      0x2002
>>
>> base-commit: aa54f5be44be786636a5d51cc1612ad208a24849
>
Amit Machhiwal Oct. 18, 2024, 1 p.m. UTC | #3 
Hi Harsh,

Thanks for looking into this patch. My comments are below:

On 2024/10/18 02:13 PM, Harsh Prateek Bora wrote:
> Hi Amit,
> 
> On 10/18/24 10:47, Harsh Prateek Bora wrote:
> > Hi Amit,
> > 
> > On 10/17/24 16:30, Amit Machhiwal wrote:
> > > The DPDES support for doorbell emulation and handling for KVM on PAPR
> > > guests was added in Linux via [1]. Subsequently, a new GSB element for
> > > DPDES was added in Linux; the same has been missing in QEMU L0. Add
> > 
> > s/QEMU L0/ TCG L0 implementation?
> > 
> > > support for DPDES register's APIv2 GSB element and required handling in
> > > `spapr_nested.c`.
> > > 
> > > Currently, booting a KVM guest inside a QEMU TCG guest fails with the
> > > below crash. The crash is encountered when GUEST_RUN_VCPU hcall made
> > > into QEMU TCG L0 fails because H_INVALID_ELEMENT_VALUE is returned as
> > > the mapping of the element ID corresponding to DPDES (unknown to QEMU
> > > TCG L0) in GSR (Guest State Request) of TCG guest's KVM to the GSB
> > > (Guest State Buffer) elements of QEMU TCG L0 fails.
> > 
> > GSB full form would be more appropriate along with first mention of GSB
> > in above text.
> > 
> > With that:
> > Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> > 
> > > 
> 
> <snip>
> 
> > > diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
> > > index c02785756c1e..b696ad537a77 100644
> > > --- a/hw/ppc/spapr_nested.c
> > > +++ b/hw/ppc/spapr_nested.c
> > > @@ -194,6 +194,7 @@ static void nested_save_state(struct
> > > nested_ppc_state *save, PowerPCCPU *cpu)
> > >           save->fscr = env->spr[SPR_FSCR];
> > >           save->pspb = env->spr[SPR_PSPB];
> > >           save->ctrl = env->spr[SPR_CTRL];
> > > +        save->dpdes = env->spr[SPR_DPDES];
> > >           save->vrsave = env->spr[SPR_VRSAVE];
> > >           save->dar = env->spr[SPR_DAR];
> > >           save->dsisr = env->spr[SPR_DSISR];
> > > @@ -293,6 +294,7 @@ static void nested_load_state(PowerPCCPU *cpu,
> > > struct nested_ppc_state *load)
> > >           env->spr[SPR_FSCR] = load->fscr;
> > >           env->spr[SPR_PSPB] = load->pspb;
> > >           env->spr[SPR_CTRL] = load->ctrl;
> > > +        env->spr[SPR_DPDES] = load->dpdes;
> > >           env->spr[SPR_VRSAVE] = load->vrsave;
> > >           env->spr[SPR_DAR] = load->dar;
> > >           env->spr[SPR_DSISR] = load->dsisr;
> 
> Looks like we overlooked above inits already present in existing code.
> I just checked above inits in nested_{save,load}_state are already
> present as common code between apiv1 & apiv2. So, we only need the inits
> below for initializing the lookup table.

The common code already has that handled, indeed. I'll make this change and the
ones you posted in the other response and send out a V2. Thanks for pointing
this out.

Thanks,
Amit

> 
> regards,
> Harsh
> > > @@ -982,6 +984,7 @@ struct guest_state_element_type
> > > guest_state_element_types[] = {
> > >       GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_FSCR,  fscr),
> > >       GUEST_STATE_ELEMENT_ENV_W(GSB_VCPU_SPR_PSPB,   pspb),
> > >       GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_CTRL,  ctrl),
> > > +    GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_DPDES, dpdes),
> > >       GUEST_STATE_ELEMENT_ENV_W(GSB_VCPU_SPR_VRSAVE, vrsave),
> > >       GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_DAR,   dar),
> > >       GUEST_STATE_ELEMENT_ENV_W(GSB_VCPU_SPR_DSISR,  dsisr),
> > > diff --git a/include/hw/ppc/spapr_nested.h
> > > b/include/hw/ppc/spapr_nested.h
> > > index 93ef14adcc5e..3b5cd993c256 100644
> > > --- a/include/hw/ppc/spapr_nested.h
> > > +++ b/include/hw/ppc/spapr_nested.h
> > > @@ -99,7 +99,8 @@
> > >   #define GSB_VCPU_SPR_HASHKEYR   0x1050
> > >   #define GSB_VCPU_SPR_HASHPKEYR  0x1051
> > >   #define GSB_VCPU_SPR_CTRL       0x1052
> > > -                    /* RESERVED 0x1053 - 0x1FFF */
> > > +#define GSB_VCPU_SPR_DPDES      0x1053
> > > +                    /* RESERVED 0x1054 - 0x1FFF */
> > >   #define GSB_VCPU_SPR_CR         0x2000
> > >   #define GSB_VCPU_SPR_PIDR       0x2001
> > >   #define GSB_VCPU_SPR_DSISR      0x2002
> > > 
> > > base-commit: aa54f5be44be786636a5d51cc1612ad208a24849
> >
diff mbox series

Patch

diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
index c02785756c1e..b696ad537a77 100644
--- a/hw/ppc/spapr_nested.c
+++ b/hw/ppc/spapr_nested.c
@@ -194,6 +194,7 @@  static void nested_save_state(struct nested_ppc_state *save, PowerPCCPU *cpu)
         save->fscr = env->spr[SPR_FSCR];
         save->pspb = env->spr[SPR_PSPB];
         save->ctrl = env->spr[SPR_CTRL];
+        save->dpdes = env->spr[SPR_DPDES];
         save->vrsave = env->spr[SPR_VRSAVE];
         save->dar = env->spr[SPR_DAR];
         save->dsisr = env->spr[SPR_DSISR];
@@ -293,6 +294,7 @@  static void nested_load_state(PowerPCCPU *cpu, struct nested_ppc_state *load)
         env->spr[SPR_FSCR] = load->fscr;
         env->spr[SPR_PSPB] = load->pspb;
         env->spr[SPR_CTRL] = load->ctrl;
+        env->spr[SPR_DPDES] = load->dpdes;
         env->spr[SPR_VRSAVE] = load->vrsave;
         env->spr[SPR_DAR] = load->dar;
         env->spr[SPR_DSISR] = load->dsisr;
@@ -982,6 +984,7 @@  struct guest_state_element_type guest_state_element_types[] = {
     GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_FSCR,  fscr),
     GUEST_STATE_ELEMENT_ENV_W(GSB_VCPU_SPR_PSPB,   pspb),
     GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_CTRL,  ctrl),
+    GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_DPDES, dpdes),
     GUEST_STATE_ELEMENT_ENV_W(GSB_VCPU_SPR_VRSAVE, vrsave),
     GUEST_STATE_ELEMENT_ENV_DW(GSB_VCPU_SPR_DAR,   dar),
     GUEST_STATE_ELEMENT_ENV_W(GSB_VCPU_SPR_DSISR,  dsisr),
diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
index 93ef14adcc5e..3b5cd993c256 100644
--- a/include/hw/ppc/spapr_nested.h
+++ b/include/hw/ppc/spapr_nested.h
@@ -99,7 +99,8 @@ 
 #define GSB_VCPU_SPR_HASHKEYR   0x1050
 #define GSB_VCPU_SPR_HASHPKEYR  0x1051
 #define GSB_VCPU_SPR_CTRL       0x1052
-                    /* RESERVED 0x1053 - 0x1FFF */
+#define GSB_VCPU_SPR_DPDES      0x1053
+                    /* RESERVED 0x1054 - 0x1FFF */
 #define GSB_VCPU_SPR_CR         0x2000
 #define GSB_VCPU_SPR_PIDR       0x2001
 #define GSB_VCPU_SPR_DSISR      0x2002