diff mbox series

[v5,28/36] ppc/xics: introduce a icp_kvm_init() routine

Message ID 20181116105729.23240-29-clg@kaod.org (mailing list archive)
State New, archived
Headers show
Series ppc: support for the XIVE interrupt controller (POWER9) | expand

Commit Message

Cédric Le Goater Nov. 16, 2018, 10:57 a.m. UTC 
This routine gathers all the KVM initialization of the XICS KVM
presenter. It will be useful when the initialization of the KVM XICS
device is moved to a global routine.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/xics_kvm.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

Comments

David Gibson Nov. 29, 2018, 4:08 a.m. UTC | #1 
On Fri, Nov 16, 2018 at 11:57:21AM +0100, Cédric Le Goater wrote:
> This routine gathers all the KVM initialization of the XICS KVM
> presenter. It will be useful when the initialization of the KVM XICS
> device is moved to a global routine.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

I dislike calling things *_init() because it's not clear which of
qemu's many "init" hooks it belongs with.

> ---
>  hw/intc/xics_kvm.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index e8fa9a53aeba..efad1b19d821 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -123,11 +123,8 @@ static void icp_kvm_reset(DeviceState *dev)
>      icp_set_kvm_state(ICP(dev), 1);
>  }
>  
> -static void icp_kvm_realize(DeviceState *dev, Error **errp)
> +static void icp_kvm_init(ICPState *icp, Error **errp)
>  {
> -    ICPState *icp = ICP(dev);
> -    ICPStateClass *icpc = ICP_GET_CLASS(icp);
> -    Error *local_err = NULL;
>      CPUState *cs;
>      KVMEnabledICP *enabled_icp;
>      unsigned long vcpu_id;
> @@ -137,12 +134,6 @@ static void icp_kvm_realize(DeviceState *dev, Error **errp)
>          abort();
>      }
>  
> -    icpc->parent_realize(dev, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> -
>      cs = icp->cs;
>      vcpu_id = kvm_arch_vcpu_id(cs);
>  
> @@ -168,6 +159,24 @@ static void icp_kvm_realize(DeviceState *dev, Error **errp)
>      QLIST_INSERT_HEAD(&kvm_enabled_icps, enabled_icp, node);
>  }
>  
> +static void icp_kvm_realize(DeviceState *dev, Error **errp)
> +{
> +    ICPStateClass *icpc = ICP_GET_CLASS(dev);
> +    Error *local_err = NULL;
> +
> +    icpc->parent_realize(dev, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    icp_kvm_init(ICP(dev), &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +}
> +
>  static void icp_kvm_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
Cédric Le Goater Nov. 29, 2018, 4:36 p.m. UTC | #2 
On 11/29/18 5:08 AM, David Gibson wrote:
> On Fri, Nov 16, 2018 at 11:57:21AM +0100, Cédric Le Goater wrote:
>> This routine gathers all the KVM initialization of the XICS KVM
>> presenter. It will be useful when the initialization of the KVM XICS
>> device is moved to a global routine.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> I dislike calling things *_init() because it's not clear which of
> qemu's many "init" hooks it belongs with.

we could use 'icp_kvm_connect' instead. Which was QEMU is doing, 
connecting a QEMU model to a KVM one.

C.

 
>> ---
>>  hw/intc/xics_kvm.c | 29 +++++++++++++++++++----------
>>  1 file changed, 19 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
>> index e8fa9a53aeba..efad1b19d821 100644
>> --- a/hw/intc/xics_kvm.c
>> +++ b/hw/intc/xics_kvm.c
>> @@ -123,11 +123,8 @@ static void icp_kvm_reset(DeviceState *dev)
>>      icp_set_kvm_state(ICP(dev), 1);
>>  }
>>  
>> -static void icp_kvm_realize(DeviceState *dev, Error **errp)
>> +static void icp_kvm_init(ICPState *icp, Error **errp)
>>  {
>> -    ICPState *icp = ICP(dev);
>> -    ICPStateClass *icpc = ICP_GET_CLASS(icp);
>> -    Error *local_err = NULL;
>>      CPUState *cs;
>>      KVMEnabledICP *enabled_icp;
>>      unsigned long vcpu_id;
>> @@ -137,12 +134,6 @@ static void icp_kvm_realize(DeviceState *dev, Error **errp)
>>          abort();
>>      }
>>  
>> -    icpc->parent_realize(dev, &local_err);
>> -    if (local_err) {
>> -        error_propagate(errp, local_err);
>> -        return;
>> -    }
>> -
>>      cs = icp->cs;
>>      vcpu_id = kvm_arch_vcpu_id(cs);
>>  
>> @@ -168,6 +159,24 @@ static void icp_kvm_realize(DeviceState *dev, Error **errp)
>>      QLIST_INSERT_HEAD(&kvm_enabled_icps, enabled_icp, node);
>>  }
>>  
>> +static void icp_kvm_realize(DeviceState *dev, Error **errp)
>> +{
>> +    ICPStateClass *icpc = ICP_GET_CLASS(dev);
>> +    Error *local_err = NULL;
>> +
>> +    icpc->parent_realize(dev, &local_err);
>> +    if (local_err) {
>> +        error_propagate(errp, local_err);
>> +        return;
>> +    }
>> +
>> +    icp_kvm_init(ICP(dev), &local_err);
>> +    if (local_err) {
>> +        error_propagate(errp, local_err);
>> +        return;
>> +    }
>> +}
>> +
>>  static void icp_kvm_class_init(ObjectClass *klass, void *data)
>>  {
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>
David Gibson Nov. 29, 2018, 10:43 p.m. UTC | #3 
On Thu, Nov 29, 2018 at 05:36:19PM +0100, Cédric Le Goater wrote:
> On 11/29/18 5:08 AM, David Gibson wrote:
> > On Fri, Nov 16, 2018 at 11:57:21AM +0100, Cédric Le Goater wrote:
> >> This routine gathers all the KVM initialization of the XICS KVM
> >> presenter. It will be useful when the initialization of the KVM XICS
> >> device is moved to a global routine.
> >>
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > 
> > I dislike calling things *_init() because it's not clear which of
> > qemu's many "init" hooks it belongs with.
> 
> we could use 'icp_kvm_connect' instead. Which was QEMU is doing, 
> connecting a QEMU model to a KVM one.

Works for me.

> 
> C.
> 
>  
> >> ---
> >>  hw/intc/xics_kvm.c | 29 +++++++++++++++++++----------
> >>  1 file changed, 19 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> >> index e8fa9a53aeba..efad1b19d821 100644
> >> --- a/hw/intc/xics_kvm.c
> >> +++ b/hw/intc/xics_kvm.c
> >> @@ -123,11 +123,8 @@ static void icp_kvm_reset(DeviceState *dev)
> >>      icp_set_kvm_state(ICP(dev), 1);
> >>  }
> >>  
> >> -static void icp_kvm_realize(DeviceState *dev, Error **errp)
> >> +static void icp_kvm_init(ICPState *icp, Error **errp)
> >>  {
> >> -    ICPState *icp = ICP(dev);
> >> -    ICPStateClass *icpc = ICP_GET_CLASS(icp);
> >> -    Error *local_err = NULL;
> >>      CPUState *cs;
> >>      KVMEnabledICP *enabled_icp;
> >>      unsigned long vcpu_id;
> >> @@ -137,12 +134,6 @@ static void icp_kvm_realize(DeviceState *dev, Error **errp)
> >>          abort();
> >>      }
> >>  
> >> -    icpc->parent_realize(dev, &local_err);
> >> -    if (local_err) {
> >> -        error_propagate(errp, local_err);
> >> -        return;
> >> -    }
> >> -
> >>      cs = icp->cs;
> >>      vcpu_id = kvm_arch_vcpu_id(cs);
> >>  
> >> @@ -168,6 +159,24 @@ static void icp_kvm_realize(DeviceState *dev, Error **errp)
> >>      QLIST_INSERT_HEAD(&kvm_enabled_icps, enabled_icp, node);
> >>  }
> >>  
> >> +static void icp_kvm_realize(DeviceState *dev, Error **errp)
> >> +{
> >> +    ICPStateClass *icpc = ICP_GET_CLASS(dev);
> >> +    Error *local_err = NULL;
> >> +
> >> +    icpc->parent_realize(dev, &local_err);
> >> +    if (local_err) {
> >> +        error_propagate(errp, local_err);
> >> +        return;
> >> +    }
> >> +
> >> +    icp_kvm_init(ICP(dev), &local_err);
> >> +    if (local_err) {
> >> +        error_propagate(errp, local_err);
> >> +        return;
> >> +    }
> >> +}
> >> +
> >>  static void icp_kvm_class_init(ObjectClass *klass, void *data)
> >>  {
> >>      DeviceClass *dc = DEVICE_CLASS(klass);
> > 
>
diff mbox series

Patch

diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index e8fa9a53aeba..efad1b19d821 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -123,11 +123,8 @@  static void icp_kvm_reset(DeviceState *dev)
     icp_set_kvm_state(ICP(dev), 1);
 }
 
-static void icp_kvm_realize(DeviceState *dev, Error **errp)
+static void icp_kvm_init(ICPState *icp, Error **errp)
 {
-    ICPState *icp = ICP(dev);
-    ICPStateClass *icpc = ICP_GET_CLASS(icp);
-    Error *local_err = NULL;
     CPUState *cs;
     KVMEnabledICP *enabled_icp;
     unsigned long vcpu_id;
@@ -137,12 +134,6 @@  static void icp_kvm_realize(DeviceState *dev, Error **errp)
         abort();
     }
 
-    icpc->parent_realize(dev, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
-
     cs = icp->cs;
     vcpu_id = kvm_arch_vcpu_id(cs);
 
@@ -168,6 +159,24 @@  static void icp_kvm_realize(DeviceState *dev, Error **errp)
     QLIST_INSERT_HEAD(&kvm_enabled_icps, enabled_icp, node);
 }
 
+static void icp_kvm_realize(DeviceState *dev, Error **errp)
+{
+    ICPStateClass *icpc = ICP_GET_CLASS(dev);
+    Error *local_err = NULL;
+
+    icpc->parent_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    icp_kvm_init(ICP(dev), &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+}
+
 static void icp_kvm_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);