[for-5.0,v5,14/23] ppc/spapr: Implement the XiveFabric interface
diff mbox series

Message ID 20191115162436.30548-15-clg@kaod.org
State New
Headers show
Series
  • ppc/pnv: add XIVE support for KVM guests
Related show

Commit Message

Cédric Le Goater Nov. 15, 2019, 4:24 p.m. UTC
The CAM line matching sequence in the pseries machine does not change
much apart from the use of the new QOM interfaces. There is an extra
indirection because of the sPAPR IRQ backend of the machine. Only the
XIVE backend implements the new 'match_nvt' handler.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/spapr.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

Comments

Greg Kurz Nov. 20, 2019, 5:53 p.m. UTC | #1
On Fri, 15 Nov 2019 17:24:27 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> The CAM line matching sequence in the pseries machine does not change
> much apart from the use of the new QOM interfaces. There is an extra
> indirection because of the sPAPR IRQ backend of the machine. Only the
> XIVE backend implements the new 'match_nvt' handler.
> 

The changelog needs an update since you dropped the indirection you had
in v4.

> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/ppc/spapr.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 94f9d27096af..a8f5850f65bb 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4270,6 +4270,39 @@ static void spapr_pic_print_info(InterruptStatsProvider *obj,
>                     kvm_irqchip_in_kernel() ? "in-kernel" : "emulated");
>  }
>  
> +static int spapr_xive_match_nvt(XiveFabric *xfb, uint8_t format,
> +                                uint8_t nvt_blk, uint32_t nvt_idx,
> +                                bool cam_ignore, uint8_t priority,
> +                                uint32_t logic_serv, XiveTCTXMatch *match)
> +{
> +    SpaprMachineState *spapr = SPAPR_MACHINE(xfb);
> +    XivePresenter *xptr = XIVE_PRESENTER(spapr->xive);
> +    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
> +    int count;
> +

As suggested by David, you should probably assert() that XIVE is in use
for extra paran^Wsafety.

With these fixed,

Reviewed-by: Greg Kurz <groug@kaod.org>

> +    count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
> +                           priority, logic_serv, match);
> +    if (count < 0) {
> +        return count;
> +    }
> +
> +    /*
> +     * When we implement the save and restore of the thread interrupt
> +     * contexts in the enter/exit CPU handlers of the machine and the
> +     * escalations in QEMU, we should be able to handle non dispatched
> +     * vCPUs.
> +     *
> +     * Until this is done, the sPAPR machine should find at least one
> +     * matching context always.
> +     */
> +    if (count == 0) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: NVT %x/%x is not dispatched\n",
> +                      nvt_blk, nvt_idx);
> +    }
> +
> +    return count;
> +}
> +
>  int spapr_get_vcpu_id(PowerPCCPU *cpu)
>  {
>      return cpu->vcpu_id;
> @@ -4366,6 +4399,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>      PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_CLASS(oc);
>      XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
>      InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
> +    XiveFabricClass *xfc = XIVE_FABRIC_CLASS(oc);
>  
>      mc->desc = "pSeries Logical Partition (PAPR compliant)";
>      mc->ignore_boot_device_suffixes = true;
> @@ -4442,6 +4476,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>      smc->linux_pci_probe = true;
>      smc->smp_threads_vsmt = true;
>      smc->nr_xirqs = SPAPR_NR_XIRQS;
> +    xfc->match_nvt = spapr_xive_match_nvt;
>  }
>  
>  static const TypeInfo spapr_machine_info = {
> @@ -4460,6 +4495,7 @@ static const TypeInfo spapr_machine_info = {
>          { TYPE_PPC_VIRTUAL_HYPERVISOR },
>          { TYPE_XICS_FABRIC },
>          { TYPE_INTERRUPT_STATS_PROVIDER },
> +        { TYPE_XIVE_FABRIC },
>          { }
>      },
>  };
Cédric Le Goater Nov. 21, 2019, 6:56 a.m. UTC | #2
On 20/11/2019 18:53, Greg Kurz wrote:
> On Fri, 15 Nov 2019 17:24:27 +0100
> Cédric Le Goater <clg@kaod.org> wrote:
> 
>> The CAM line matching sequence in the pseries machine does not change
>> much apart from the use of the new QOM interfaces. There is an extra
>> indirection because of the sPAPR IRQ backend of the machine. Only the
>> XIVE backend implements the new 'match_nvt' handler.
>>
> 
> The changelog needs an update since you dropped the indirection you had
> in v4.

Indeed.

> 
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  hw/ppc/spapr.c | 36 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 36 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 94f9d27096af..a8f5850f65bb 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -4270,6 +4270,39 @@ static void spapr_pic_print_info(InterruptStatsProvider *obj,
>>                     kvm_irqchip_in_kernel() ? "in-kernel" : "emulated");
>>  }
>>  
>> +static int spapr_xive_match_nvt(XiveFabric *xfb, uint8_t format,
>> +                                uint8_t nvt_blk, uint32_t nvt_idx,
>> +                                bool cam_ignore, uint8_t priority,
>> +                                uint32_t logic_serv, XiveTCTXMatch *match)
>> +{
>> +    SpaprMachineState *spapr = SPAPR_MACHINE(xfb);
>> +    XivePresenter *xptr = XIVE_PRESENTER(spapr->xive);
>> +    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
>> +    int count;
>> +
> 
> As suggested by David, you should probably assert() that XIVE is in use
> for extra paran^Wsafety.

I don't see the need. The stack call is clear enough IMO. It can only be 
reached from the XiveRouter.

Thanks,

C. 

> With these fixed,
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
>> +    count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
>> +                           priority, logic_serv, match);
>> +    if (count < 0) {
>> +        return count;
>> +    }
>> +
>> +    /*
>> +     * When we implement the save and restore of the thread interrupt
>> +     * contexts in the enter/exit CPU handlers of the machine and the
>> +     * escalations in QEMU, we should be able to handle non dispatched
>> +     * vCPUs.
>> +     *
>> +     * Until this is done, the sPAPR machine should find at least one
>> +     * matching context always.
>> +     */
>> +    if (count == 0) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: NVT %x/%x is not dispatched\n",
>> +                      nvt_blk, nvt_idx);
>> +    }
>> +
>> +    return count;
>> +}
>> +
>>  int spapr_get_vcpu_id(PowerPCCPU *cpu)
>>  {
>>      return cpu->vcpu_id;
>> @@ -4366,6 +4399,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>>      PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_CLASS(oc);
>>      XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
>>      InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
>> +    XiveFabricClass *xfc = XIVE_FABRIC_CLASS(oc);
>>  
>>      mc->desc = "pSeries Logical Partition (PAPR compliant)";
>>      mc->ignore_boot_device_suffixes = true;
>> @@ -4442,6 +4476,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>>      smc->linux_pci_probe = true;
>>      smc->smp_threads_vsmt = true;
>>      smc->nr_xirqs = SPAPR_NR_XIRQS;
>> +    xfc->match_nvt = spapr_xive_match_nvt;
>>  }
>>  
>>  static const TypeInfo spapr_machine_info = {
>> @@ -4460,6 +4495,7 @@ static const TypeInfo spapr_machine_info = {
>>          { TYPE_PPC_VIRTUAL_HYPERVISOR },
>>          { TYPE_XICS_FABRIC },
>>          { TYPE_INTERRUPT_STATS_PROVIDER },
>> +        { TYPE_XIVE_FABRIC },
>>          { }
>>      },
>>  };
>
Greg Kurz Nov. 21, 2019, 7:24 a.m. UTC | #3
On Thu, 21 Nov 2019 07:56:32 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> On 20/11/2019 18:53, Greg Kurz wrote:
> > On Fri, 15 Nov 2019 17:24:27 +0100
> > Cédric Le Goater <clg@kaod.org> wrote:
> > 
> >> The CAM line matching sequence in the pseries machine does not change
> >> much apart from the use of the new QOM interfaces. There is an extra
> >> indirection because of the sPAPR IRQ backend of the machine. Only the
> >> XIVE backend implements the new 'match_nvt' handler.
> >>
> > 
> > The changelog needs an update since you dropped the indirection you had
> > in v4.
> 
> Indeed.
> 
> > 
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >> ---
> >>  hw/ppc/spapr.c | 36 ++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 36 insertions(+)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index 94f9d27096af..a8f5850f65bb 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -4270,6 +4270,39 @@ static void spapr_pic_print_info(InterruptStatsProvider *obj,
> >>                     kvm_irqchip_in_kernel() ? "in-kernel" : "emulated");
> >>  }
> >>  
> >> +static int spapr_xive_match_nvt(XiveFabric *xfb, uint8_t format,
> >> +                                uint8_t nvt_blk, uint32_t nvt_idx,
> >> +                                bool cam_ignore, uint8_t priority,
> >> +                                uint32_t logic_serv, XiveTCTXMatch *match)
> >> +{
> >> +    SpaprMachineState *spapr = SPAPR_MACHINE(xfb);
> >> +    XivePresenter *xptr = XIVE_PRESENTER(spapr->xive);
> >> +    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
> >> +    int count;
> >> +
> > 
> > As suggested by David, you should probably assert() that XIVE is in use
> > for extra paran^Wsafety.
> 
> I don't see the need. The stack call is clear enough IMO. It can only be 
> reached from the XiveRouter.
> 

Hmm... the assert() proposal isn't about this getting called by some
other code, it is about ensuring XIVE is the active IC in case the
machine was started with ic-mode=dual. But if you're confident enough
it can never ever happen, no matter any subsequent change may done to
the code, then don't add it :)

> Thanks,
> 
> C. 
> 
> > With these fixed,
> > 
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > 
> >> +    count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
> >> +                           priority, logic_serv, match);
> >> +    if (count < 0) {
> >> +        return count;
> >> +    }
> >> +
> >> +    /*
> >> +     * When we implement the save and restore of the thread interrupt
> >> +     * contexts in the enter/exit CPU handlers of the machine and the
> >> +     * escalations in QEMU, we should be able to handle non dispatched
> >> +     * vCPUs.
> >> +     *
> >> +     * Until this is done, the sPAPR machine should find at least one
> >> +     * matching context always.
> >> +     */
> >> +    if (count == 0) {
> >> +        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: NVT %x/%x is not dispatched\n",
> >> +                      nvt_blk, nvt_idx);
> >> +    }
> >> +
> >> +    return count;
> >> +}
> >> +
> >>  int spapr_get_vcpu_id(PowerPCCPU *cpu)
> >>  {
> >>      return cpu->vcpu_id;
> >> @@ -4366,6 +4399,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> >>      PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_CLASS(oc);
> >>      XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
> >>      InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
> >> +    XiveFabricClass *xfc = XIVE_FABRIC_CLASS(oc);
> >>  
> >>      mc->desc = "pSeries Logical Partition (PAPR compliant)";
> >>      mc->ignore_boot_device_suffixes = true;
> >> @@ -4442,6 +4476,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> >>      smc->linux_pci_probe = true;
> >>      smc->smp_threads_vsmt = true;
> >>      smc->nr_xirqs = SPAPR_NR_XIRQS;
> >> +    xfc->match_nvt = spapr_xive_match_nvt;
> >>  }
> >>  
> >>  static const TypeInfo spapr_machine_info = {
> >> @@ -4460,6 +4495,7 @@ static const TypeInfo spapr_machine_info = {
> >>          { TYPE_PPC_VIRTUAL_HYPERVISOR },
> >>          { TYPE_XICS_FABRIC },
> >>          { TYPE_INTERRUPT_STATS_PROVIDER },
> >> +        { TYPE_XIVE_FABRIC },
> >>          { }
> >>      },
> >>  };
> > 
>
Cédric Le Goater Nov. 21, 2019, 7:38 a.m. UTC | #4
On 21/11/2019 08:24, Greg Kurz wrote:
> On Thu, 21 Nov 2019 07:56:32 +0100
> Cédric Le Goater <clg@kaod.org> wrote:
> 
>> On 20/11/2019 18:53, Greg Kurz wrote:
>>> On Fri, 15 Nov 2019 17:24:27 +0100
>>> Cédric Le Goater <clg@kaod.org> wrote:
>>>
>>>> The CAM line matching sequence in the pseries machine does not change
>>>> much apart from the use of the new QOM interfaces. There is an extra
>>>> indirection because of the sPAPR IRQ backend of the machine. Only the
>>>> XIVE backend implements the new 'match_nvt' handler.
>>>>
>>>
>>> The changelog needs an update since you dropped the indirection you had
>>> in v4.
>>
>> Indeed.
>>
>>>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> ---
>>>>  hw/ppc/spapr.c | 36 ++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 36 insertions(+)
>>>>
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index 94f9d27096af..a8f5850f65bb 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -4270,6 +4270,39 @@ static void spapr_pic_print_info(InterruptStatsProvider *obj,
>>>>                     kvm_irqchip_in_kernel() ? "in-kernel" : "emulated");
>>>>  }
>>>>  
>>>> +static int spapr_xive_match_nvt(XiveFabric *xfb, uint8_t format,
>>>> +                                uint8_t nvt_blk, uint32_t nvt_idx,
>>>> +                                bool cam_ignore, uint8_t priority,
>>>> +                                uint32_t logic_serv, XiveTCTXMatch *match)
>>>> +{
>>>> +    SpaprMachineState *spapr = SPAPR_MACHINE(xfb);
>>>> +    XivePresenter *xptr = XIVE_PRESENTER(spapr->xive);
>>>> +    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
>>>> +    int count;
>>>> +
>>>
>>> As suggested by David, you should probably assert() that XIVE is in use
>>> for extra paran^Wsafety.
>>
>> I don't see the need. The stack call is clear enough IMO. It can only be 
>> reached from the XiveRouter.
>>
> 
> Hmm... the assert() proposal isn't about this getting called by some
> other code, it is about ensuring XIVE is the active IC in case the
> machine was started with ic-mode=dual. But if you're confident enough
> it can never ever happen, no matter any subsequent change may done to
> the code, then don't add it :)

If XIVE mode is not selected, the XIVE ESB pages are not mapped in the 
machine address space and you can not reach the Router without them.

C.

> 
>> Thanks,
>>
>> C. 
>>
>>> With these fixed,
>>>
>>> Reviewed-by: Greg Kurz <groug@kaod.org>
>>>
>>>> +    count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
>>>> +                           priority, logic_serv, match);
>>>> +    if (count < 0) {
>>>> +        return count;
>>>> +    }
>>>> +
>>>> +    /*
>>>> +     * When we implement the save and restore of the thread interrupt
>>>> +     * contexts in the enter/exit CPU handlers of the machine and the
>>>> +     * escalations in QEMU, we should be able to handle non dispatched
>>>> +     * vCPUs.
>>>> +     *
>>>> +     * Until this is done, the sPAPR machine should find at least one
>>>> +     * matching context always.
>>>> +     */
>>>> +    if (count == 0) {
>>>> +        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: NVT %x/%x is not dispatched\n",
>>>> +                      nvt_blk, nvt_idx);
>>>> +    }
>>>> +
>>>> +    return count;
>>>> +}
>>>> +
>>>>  int spapr_get_vcpu_id(PowerPCCPU *cpu)
>>>>  {
>>>>      return cpu->vcpu_id;
>>>> @@ -4366,6 +4399,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>>>>      PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_CLASS(oc);
>>>>      XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
>>>>      InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
>>>> +    XiveFabricClass *xfc = XIVE_FABRIC_CLASS(oc);
>>>>  
>>>>      mc->desc = "pSeries Logical Partition (PAPR compliant)";
>>>>      mc->ignore_boot_device_suffixes = true;
>>>> @@ -4442,6 +4476,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>>>>      smc->linux_pci_probe = true;
>>>>      smc->smp_threads_vsmt = true;
>>>>      smc->nr_xirqs = SPAPR_NR_XIRQS;
>>>> +    xfc->match_nvt = spapr_xive_match_nvt;
>>>>  }
>>>>  
>>>>  static const TypeInfo spapr_machine_info = {
>>>> @@ -4460,6 +4495,7 @@ static const TypeInfo spapr_machine_info = {
>>>>          { TYPE_PPC_VIRTUAL_HYPERVISOR },
>>>>          { TYPE_XICS_FABRIC },
>>>>          { TYPE_INTERRUPT_STATS_PROVIDER },
>>>> +        { TYPE_XIVE_FABRIC },
>>>>          { }
>>>>      },
>>>>  };
>>>
>>
>

Patch
diff mbox series

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 94f9d27096af..a8f5850f65bb 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4270,6 +4270,39 @@  static void spapr_pic_print_info(InterruptStatsProvider *obj,
                    kvm_irqchip_in_kernel() ? "in-kernel" : "emulated");
 }
 
+static int spapr_xive_match_nvt(XiveFabric *xfb, uint8_t format,
+                                uint8_t nvt_blk, uint32_t nvt_idx,
+                                bool cam_ignore, uint8_t priority,
+                                uint32_t logic_serv, XiveTCTXMatch *match)
+{
+    SpaprMachineState *spapr = SPAPR_MACHINE(xfb);
+    XivePresenter *xptr = XIVE_PRESENTER(spapr->xive);
+    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
+    int count;
+
+    count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
+                           priority, logic_serv, match);
+    if (count < 0) {
+        return count;
+    }
+
+    /*
+     * When we implement the save and restore of the thread interrupt
+     * contexts in the enter/exit CPU handlers of the machine and the
+     * escalations in QEMU, we should be able to handle non dispatched
+     * vCPUs.
+     *
+     * Until this is done, the sPAPR machine should find at least one
+     * matching context always.
+     */
+    if (count == 0) {
+        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: NVT %x/%x is not dispatched\n",
+                      nvt_blk, nvt_idx);
+    }
+
+    return count;
+}
+
 int spapr_get_vcpu_id(PowerPCCPU *cpu)
 {
     return cpu->vcpu_id;
@@ -4366,6 +4399,7 @@  static void spapr_machine_class_init(ObjectClass *oc, void *data)
     PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_CLASS(oc);
     XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
     InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
+    XiveFabricClass *xfc = XIVE_FABRIC_CLASS(oc);
 
     mc->desc = "pSeries Logical Partition (PAPR compliant)";
     mc->ignore_boot_device_suffixes = true;
@@ -4442,6 +4476,7 @@  static void spapr_machine_class_init(ObjectClass *oc, void *data)
     smc->linux_pci_probe = true;
     smc->smp_threads_vsmt = true;
     smc->nr_xirqs = SPAPR_NR_XIRQS;
+    xfc->match_nvt = spapr_xive_match_nvt;
 }
 
 static const TypeInfo spapr_machine_info = {
@@ -4460,6 +4495,7 @@  static const TypeInfo spapr_machine_info = {
         { TYPE_PPC_VIRTUAL_HYPERVISOR },
         { TYPE_XICS_FABRIC },
         { TYPE_INTERRUPT_STATS_PROVIDER },
+        { TYPE_XIVE_FABRIC },
         { }
     },
 };