| Message ID | 155023080049.1011724.15423463482790260696.stgit@bahia.lan (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xics: Get rid of KVM specific classes | expand |
On 2/15/19 12:40 PM, Greg Kurz wrote: > The realization of KVM ICP currently follows the parent_realize logic, > which is a bit overkill here. Also we want to get rid of the KVM ICP > class. Explicitely call icp_kvm_realize() from the base ICP realize > function. > > Note that ICPStateClass::parent_realize is retained because powernv > needs it. > > Signed-off-by: Greg Kurz <groug@kaod.org>> > --- > hw/intc/xics.c | 8 ++++++++ > hw/intc/xics_kvm.c | 10 +--------- > include/hw/ppc/xics.h | 1 + > 3 files changed, 10 insertions(+), 9 deletions(-) > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index 822d367e6388..acd63ab5e0b9 100644 > --- a/hw/intc/xics.c > +++ b/hw/intc/xics.c > @@ -349,6 +349,14 @@ static void icp_realize(DeviceState *dev, Error **errp) > return; > } > > + if (kvm_irqchip_in_kernel()) { > + icp_kvm_realize(dev, &err); While we are at changing things, I would prefix all the KVM backends routine with kvmppc_*. so that icp_kvm_realize() becomes kvmppc_icp_realize() Apart from that, Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > + if (err) { > + error_propagate(errp, err); > + return; > + } > + } > + > qemu_register_reset(icp_reset_handler, dev); > vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp); > } > diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c > index 80321e9b75ab..4eebced516b6 100644 > --- a/hw/intc/xics_kvm.c > +++ b/hw/intc/xics_kvm.c > @@ -115,11 +115,9 @@ int icp_set_kvm_state(ICPState *icp) > return 0; > } > > -static void icp_kvm_realize(DeviceState *dev, Error **errp) > +void icp_kvm_realize(DeviceState *dev, 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; > @@ -129,12 +127,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); > > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index e33282a576d0..ab61dc24010a 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -202,5 +202,6 @@ Object *icp_create(Object *cpu, const char *type, XICSFabric *xi, > void icp_get_kvm_state(ICPState *icp); > int icp_set_kvm_state(ICPState *icp); > void icp_synchronize_state(ICPState *icp); > +void icp_kvm_realize(DeviceState *dev, Error **errp); > > #endif /* XICS_H */ >
On Fri, 15 Feb 2019 13:54:02 +0100 Cédric Le Goater <clg@kaod.org> wrote: > On 2/15/19 12:40 PM, Greg Kurz wrote: > > The realization of KVM ICP currently follows the parent_realize logic, > > which is a bit overkill here. Also we want to get rid of the KVM ICP > > class. Explicitely call icp_kvm_realize() from the base ICP realize > > function. > > > > Note that ICPStateClass::parent_realize is retained because powernv > > needs it. > > > > Signed-off-by: Greg Kurz <groug@kaod.org>> > > --- > > hw/intc/xics.c | 8 ++++++++ > > hw/intc/xics_kvm.c | 10 +--------- > > include/hw/ppc/xics.h | 1 + > > 3 files changed, 10 insertions(+), 9 deletions(-) > > > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > > index 822d367e6388..acd63ab5e0b9 100644 > > --- a/hw/intc/xics.c > > +++ b/hw/intc/xics.c > > @@ -349,6 +349,14 @@ static void icp_realize(DeviceState *dev, Error **errp) > > return; > > } > > > > + if (kvm_irqchip_in_kernel()) { > > + icp_kvm_realize(dev, &err); > > While we are at changing things, I would prefix all the KVM > backends routine with kvmppc_*. so that icp_kvm_realize() > becomes kvmppc_icp_realize() > Well... kvmppc_* routines have historically been sitting under target/ppc so I'm not sure we want to use the same prefix elsewhere... > Apart from that, > > Reviewed-by: Cédric Le Goater <clg@kaod.org> > > Thanks, > > C. > > > > + if (err) { > > + error_propagate(errp, err); > > + return; > > + } > > + } > > + > > qemu_register_reset(icp_reset_handler, dev); > > vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp); > > } > > diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c > > index 80321e9b75ab..4eebced516b6 100644 > > --- a/hw/intc/xics_kvm.c > > +++ b/hw/intc/xics_kvm.c > > @@ -115,11 +115,9 @@ int icp_set_kvm_state(ICPState *icp) > > return 0; > > } > > > > -static void icp_kvm_realize(DeviceState *dev, Error **errp) > > +void icp_kvm_realize(DeviceState *dev, 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; > > @@ -129,12 +127,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); > > > > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > > index e33282a576d0..ab61dc24010a 100644 > > --- a/include/hw/ppc/xics.h > > +++ b/include/hw/ppc/xics.h > > @@ -202,5 +202,6 @@ Object *icp_create(Object *cpu, const char *type, XICSFabric *xi, > > void icp_get_kvm_state(ICPState *icp); > > int icp_set_kvm_state(ICPState *icp); > > void icp_synchronize_state(ICPState *icp); > > +void icp_kvm_realize(DeviceState *dev, Error **errp); > > > > #endif /* XICS_H */ > > >
On 2/15/19 2:03 PM, Greg Kurz wrote: > On Fri, 15 Feb 2019 13:54:02 +0100 > Cédric Le Goater <clg@kaod.org> wrote: > >> On 2/15/19 12:40 PM, Greg Kurz wrote: >>> The realization of KVM ICP currently follows the parent_realize logic, >>> which is a bit overkill here. Also we want to get rid of the KVM ICP >>> class. Explicitely call icp_kvm_realize() from the base ICP realize >>> function. >>> >>> Note that ICPStateClass::parent_realize is retained because powernv >>> needs it. >>> >>> Signed-off-by: Greg Kurz <groug@kaod.org>> >>> --- >>> hw/intc/xics.c | 8 ++++++++ >>> hw/intc/xics_kvm.c | 10 +--------- >>> include/hw/ppc/xics.h | 1 + >>> 3 files changed, 10 insertions(+), 9 deletions(-) >>> >>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c >>> index 822d367e6388..acd63ab5e0b9 100644 >>> --- a/hw/intc/xics.c >>> +++ b/hw/intc/xics.c >>> @@ -349,6 +349,14 @@ static void icp_realize(DeviceState *dev, Error **errp) >>> return; >>> } >>> >>> + if (kvm_irqchip_in_kernel()) { >>> + icp_kvm_realize(dev, &err); >> >> While we are at changing things, I would prefix all the KVM >> backends routine with kvmppc_*. so that icp_kvm_realize() >> becomes kvmppc_icp_realize() >> > > Well... kvmppc_* routines have historically been sitting under > target/ppc so I'm not sure we want to use the same prefix > elsewhere... Well, they could also be moved there but I think what is important is that the kvmppc_* routine should be used under the kvm_enabled() flag. Those under target/ppc have and extra dummy stub provided for the !kvm_enabled() case. C. > >> Apart from that, >> >> Reviewed-by: Cédric Le Goater <clg@kaod.org> >> >> Thanks, >> >> C. >> >> >>> + if (err) { >>> + error_propagate(errp, err); >>> + return; >>> + } >>> + } >>> + >>> qemu_register_reset(icp_reset_handler, dev); >>> vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp); >>> } >>> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c >>> index 80321e9b75ab..4eebced516b6 100644 >>> --- a/hw/intc/xics_kvm.c >>> +++ b/hw/intc/xics_kvm.c >>> @@ -115,11 +115,9 @@ int icp_set_kvm_state(ICPState *icp) >>> return 0; >>> } >>> >>> -static void icp_kvm_realize(DeviceState *dev, Error **errp) >>> +void icp_kvm_realize(DeviceState *dev, 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; >>> @@ -129,12 +127,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); >>> >>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h >>> index e33282a576d0..ab61dc24010a 100644 >>> --- a/include/hw/ppc/xics.h >>> +++ b/include/hw/ppc/xics.h >>> @@ -202,5 +202,6 @@ Object *icp_create(Object *cpu, const char *type, XICSFabric *xi, >>> void icp_get_kvm_state(ICPState *icp); >>> int icp_set_kvm_state(ICPState *icp); >>> void icp_synchronize_state(ICPState *icp); >>> +void icp_kvm_realize(DeviceState *dev, Error **errp); >>> >>> #endif /* XICS_H */ >>> >> >
On Fri, 15 Feb 2019 14:09:53 +0100 Cédric Le Goater <clg@kaod.org> wrote: > On 2/15/19 2:03 PM, Greg Kurz wrote: > > On Fri, 15 Feb 2019 13:54:02 +0100 > > Cédric Le Goater <clg@kaod.org> wrote: > > > >> On 2/15/19 12:40 PM, Greg Kurz wrote: > >>> The realization of KVM ICP currently follows the parent_realize logic, > >>> which is a bit overkill here. Also we want to get rid of the KVM ICP > >>> class. Explicitely call icp_kvm_realize() from the base ICP realize > >>> function. > >>> > >>> Note that ICPStateClass::parent_realize is retained because powernv > >>> needs it. > >>> > >>> Signed-off-by: Greg Kurz <groug@kaod.org>> > >>> --- > >>> hw/intc/xics.c | 8 ++++++++ > >>> hw/intc/xics_kvm.c | 10 +--------- > >>> include/hw/ppc/xics.h | 1 + > >>> 3 files changed, 10 insertions(+), 9 deletions(-) > >>> > >>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c > >>> index 822d367e6388..acd63ab5e0b9 100644 > >>> --- a/hw/intc/xics.c > >>> +++ b/hw/intc/xics.c > >>> @@ -349,6 +349,14 @@ static void icp_realize(DeviceState *dev, Error **errp) > >>> return; > >>> } > >>> > >>> + if (kvm_irqchip_in_kernel()) { > >>> + icp_kvm_realize(dev, &err); > >> > >> While we are at changing things, I would prefix all the KVM > >> backends routine with kvmppc_*. so that icp_kvm_realize() > >> becomes kvmppc_icp_realize() > >> > > > > Well... kvmppc_* routines have historically been sitting under > > target/ppc so I'm not sure we want to use the same prefix > > elsewhere... > > Well, they could also be moved there but I think what is important > is that the kvmppc_* routine should be used under the kvm_enabled() > flag. > > Those under target/ppc have and extra dummy stub provided for the > !kvm_enabled() case. > Well, I don't really care but if we go this way (David?), I'd rather do it globally in a followup patch. > C. > > > > > > >> Apart from that, > >> > >> Reviewed-by: Cédric Le Goater <clg@kaod.org> > >> > >> Thanks, > >> > >> C. > >> > >> > >>> + if (err) { > >>> + error_propagate(errp, err); > >>> + return; > >>> + } > >>> + } > >>> + > >>> qemu_register_reset(icp_reset_handler, dev); > >>> vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp); > >>> } > >>> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c > >>> index 80321e9b75ab..4eebced516b6 100644 > >>> --- a/hw/intc/xics_kvm.c > >>> +++ b/hw/intc/xics_kvm.c > >>> @@ -115,11 +115,9 @@ int icp_set_kvm_state(ICPState *icp) > >>> return 0; > >>> } > >>> > >>> -static void icp_kvm_realize(DeviceState *dev, Error **errp) > >>> +void icp_kvm_realize(DeviceState *dev, 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; > >>> @@ -129,12 +127,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); > >>> > >>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > >>> index e33282a576d0..ab61dc24010a 100644 > >>> --- a/include/hw/ppc/xics.h > >>> +++ b/include/hw/ppc/xics.h > >>> @@ -202,5 +202,6 @@ Object *icp_create(Object *cpu, const char *type, XICSFabric *xi, > >>> void icp_get_kvm_state(ICPState *icp); > >>> int icp_set_kvm_state(ICPState *icp); > >>> void icp_synchronize_state(ICPState *icp); > >>> +void icp_kvm_realize(DeviceState *dev, Error **errp); > >>> > >>> #endif /* XICS_H */ > >>> > >> > > >
On 2/15/19 2:27 PM, Greg Kurz wrote: > On Fri, 15 Feb 2019 14:09:53 +0100 > Cédric Le Goater <clg@kaod.org> wrote: > >> On 2/15/19 2:03 PM, Greg Kurz wrote: >>> On Fri, 15 Feb 2019 13:54:02 +0100 >>> Cédric Le Goater <clg@kaod.org> wrote: >>> >>>> On 2/15/19 12:40 PM, Greg Kurz wrote: >>>>> The realization of KVM ICP currently follows the parent_realize logic, >>>>> which is a bit overkill here. Also we want to get rid of the KVM ICP >>>>> class. Explicitely call icp_kvm_realize() from the base ICP realize >>>>> function. >>>>> >>>>> Note that ICPStateClass::parent_realize is retained because powernv >>>>> needs it. >>>>> >>>>> Signed-off-by: Greg Kurz <groug@kaod.org>> >>>>> --- >>>>> hw/intc/xics.c | 8 ++++++++ >>>>> hw/intc/xics_kvm.c | 10 +--------- >>>>> include/hw/ppc/xics.h | 1 + >>>>> 3 files changed, 10 insertions(+), 9 deletions(-) >>>>> >>>>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c >>>>> index 822d367e6388..acd63ab5e0b9 100644 >>>>> --- a/hw/intc/xics.c >>>>> +++ b/hw/intc/xics.c >>>>> @@ -349,6 +349,14 @@ static void icp_realize(DeviceState *dev, Error **errp) >>>>> return; >>>>> } >>>>> >>>>> + if (kvm_irqchip_in_kernel()) { >>>>> + icp_kvm_realize(dev, &err); >>>> >>>> While we are at changing things, I would prefix all the KVM >>>> backends routine with kvmppc_*. so that icp_kvm_realize() >>>> becomes kvmppc_icp_realize() >>>> >>> >>> Well... kvmppc_* routines have historically been sitting under >>> target/ppc so I'm not sure we want to use the same prefix >>> elsewhere... >> >> Well, they could also be moved there but I think what is important >> is that the kvmppc_* routine should be used under the kvm_enabled() >> flag. >> >> Those under target/ppc have and extra dummy stub provided for the >> !kvm_enabled() case. >> > > Well, I don't really care but if we go this way (David?), I'd rather do it > globally in a followup patch. yes. that's fine also. C. > >> C. >> >> >> >>> >>>> Apart from that, >>>> >>>> Reviewed-by: Cédric Le Goater <clg@kaod.org> >>>> >>>> Thanks, >>>> >>>> C. >>>> >>>> >>>>> + if (err) { >>>>> + error_propagate(errp, err); >>>>> + return; >>>>> + } >>>>> + } >>>>> + >>>>> qemu_register_reset(icp_reset_handler, dev); >>>>> vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp); >>>>> } >>>>> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c >>>>> index 80321e9b75ab..4eebced516b6 100644 >>>>> --- a/hw/intc/xics_kvm.c >>>>> +++ b/hw/intc/xics_kvm.c >>>>> @@ -115,11 +115,9 @@ int icp_set_kvm_state(ICPState *icp) >>>>> return 0; >>>>> } >>>>> >>>>> -static void icp_kvm_realize(DeviceState *dev, Error **errp) >>>>> +void icp_kvm_realize(DeviceState *dev, 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; >>>>> @@ -129,12 +127,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); >>>>> >>>>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h >>>>> index e33282a576d0..ab61dc24010a 100644 >>>>> --- a/include/hw/ppc/xics.h >>>>> +++ b/include/hw/ppc/xics.h >>>>> @@ -202,5 +202,6 @@ Object *icp_create(Object *cpu, const char *type, XICSFabric *xi, >>>>> void icp_get_kvm_state(ICPState *icp); >>>>> int icp_set_kvm_state(ICPState *icp); >>>>> void icp_synchronize_state(ICPState *icp); >>>>> +void icp_kvm_realize(DeviceState *dev, Error **errp); >>>>> >>>>> #endif /* XICS_H */ >>>>> >>>> >>> >> >
On Fri, Feb 15, 2019 at 02:27:41PM +0100, Greg Kurz wrote: > On Fri, 15 Feb 2019 14:09:53 +0100 > Cédric Le Goater <clg@kaod.org> wrote: > > > On 2/15/19 2:03 PM, Greg Kurz wrote: > > > On Fri, 15 Feb 2019 13:54:02 +0100 > > > Cédric Le Goater <clg@kaod.org> wrote: > > > > > >> On 2/15/19 12:40 PM, Greg Kurz wrote: > > >>> The realization of KVM ICP currently follows the parent_realize logic, > > >>> which is a bit overkill here. Also we want to get rid of the KVM ICP > > >>> class. Explicitely call icp_kvm_realize() from the base ICP realize > > >>> function. > > >>> > > >>> Note that ICPStateClass::parent_realize is retained because powernv > > >>> needs it. > > >>> > > >>> Signed-off-by: Greg Kurz <groug@kaod.org>> > > >>> --- > > >>> hw/intc/xics.c | 8 ++++++++ > > >>> hw/intc/xics_kvm.c | 10 +--------- > > >>> include/hw/ppc/xics.h | 1 + > > >>> 3 files changed, 10 insertions(+), 9 deletions(-) > > >>> > > >>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c > > >>> index 822d367e6388..acd63ab5e0b9 100644 > > >>> --- a/hw/intc/xics.c > > >>> +++ b/hw/intc/xics.c > > >>> @@ -349,6 +349,14 @@ static void icp_realize(DeviceState *dev, Error **errp) > > >>> return; > > >>> } > > >>> > > >>> + if (kvm_irqchip_in_kernel()) { > > >>> + icp_kvm_realize(dev, &err); > > >> > > >> While we are at changing things, I would prefix all the KVM > > >> backends routine with kvmppc_*. so that icp_kvm_realize() > > >> becomes kvmppc_icp_realize() > > >> > > > > > > Well... kvmppc_* routines have historically been sitting under > > > target/ppc so I'm not sure we want to use the same prefix > > > elsewhere... > > > > Well, they could also be moved there but I think what is important > > is that the kvmppc_* routine should be used under the kvm_enabled() > > flag. > > > > Those under target/ppc have and extra dummy stub provided for the > > !kvm_enabled() case. > > > > Well, I don't really care but if we go this way (David?), I'd rather do it > globally in a followup patch. I concur. I'm not all that fussed really, and I think it would be done best as a followup. > > > C. > > > > > > > > > > > >> Apart from that, > > >> > > >> Reviewed-by: Cédric Le Goater <clg@kaod.org> > > >> > > >> Thanks, > > >> > > >> C. > > >> > > >> > > >>> + if (err) { > > >>> + error_propagate(errp, err); > > >>> + return; > > >>> + } > > >>> + } > > >>> + > > >>> qemu_register_reset(icp_reset_handler, dev); > > >>> vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp); > > >>> } > > >>> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c > > >>> index 80321e9b75ab..4eebced516b6 100644 > > >>> --- a/hw/intc/xics_kvm.c > > >>> +++ b/hw/intc/xics_kvm.c > > >>> @@ -115,11 +115,9 @@ int icp_set_kvm_state(ICPState *icp) > > >>> return 0; > > >>> } > > >>> > > >>> -static void icp_kvm_realize(DeviceState *dev, Error **errp) > > >>> +void icp_kvm_realize(DeviceState *dev, 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; > > >>> @@ -129,12 +127,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); > > >>> > > >>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > > >>> index e33282a576d0..ab61dc24010a 100644 > > >>> --- a/include/hw/ppc/xics.h > > >>> +++ b/include/hw/ppc/xics.h > > >>> @@ -202,5 +202,6 @@ Object *icp_create(Object *cpu, const char *type, XICSFabric *xi, > > >>> void icp_get_kvm_state(ICPState *icp); > > >>> int icp_set_kvm_state(ICPState *icp); > > >>> void icp_synchronize_state(ICPState *icp); > > >>> +void icp_kvm_realize(DeviceState *dev, Error **errp); > > >>> > > >>> #endif /* XICS_H */ > > >>> > > >> > > > > > >
diff --git a/hw/intc/xics.c b/hw/intc/xics.c index 822d367e6388..acd63ab5e0b9 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -349,6 +349,14 @@ static void icp_realize(DeviceState *dev, Error **errp) return; } + if (kvm_irqchip_in_kernel()) { + icp_kvm_realize(dev, &err); + if (err) { + error_propagate(errp, err); + return; + } + } + qemu_register_reset(icp_reset_handler, dev); vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp); } diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c index 80321e9b75ab..4eebced516b6 100644 --- a/hw/intc/xics_kvm.c +++ b/hw/intc/xics_kvm.c @@ -115,11 +115,9 @@ int icp_set_kvm_state(ICPState *icp) return 0; } -static void icp_kvm_realize(DeviceState *dev, Error **errp) +void icp_kvm_realize(DeviceState *dev, 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; @@ -129,12 +127,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); diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h index e33282a576d0..ab61dc24010a 100644 --- a/include/hw/ppc/xics.h +++ b/include/hw/ppc/xics.h @@ -202,5 +202,6 @@ Object *icp_create(Object *cpu, const char *type, XICSFabric *xi, void icp_get_kvm_state(ICPState *icp); int icp_set_kvm_state(ICPState *icp); void icp_synchronize_state(ICPState *icp); +void icp_kvm_realize(DeviceState *dev, Error **errp); #endif /* XICS_H */
The realization of KVM ICP currently follows the parent_realize logic, which is a bit overkill here. Also we want to get rid of the KVM ICP class. Explicitely call icp_kvm_realize() from the base ICP realize function. Note that ICPStateClass::parent_realize is retained because powernv needs it. Signed-off-by: Greg Kurz <groug@kaod.org> --- hw/intc/xics.c | 8 ++++++++ hw/intc/xics_kvm.c | 10 +--------- include/hw/ppc/xics.h | 1 + 3 files changed, 10 insertions(+), 9 deletions(-)