| Message ID | 1527750994-14360-1-git-send-email-zhaoshenglong@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: GIC: Fix memory leak due to calling kvm_init_irq_routing twice | expand |
Hi Shannon, On 05/31/2018 09:16 AM, Shannon Zhao wrote: > kvm_irqchip_create called by kvm_init will call kvm_init_irq_routing to > initialize global capability variables. If we call kvm_init_irq_routing in > GIC realize function, previous allocated memory will leak. > > Fix this by deleting the unnecessary call. > > Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> openpic_kvm seems to suffer the same leak. Don't you want to fix it as well? Reviewed-by: Eric Auger <eric.auger@redhat.com> Thanks Eric > --- > hw/intc/arm_gic_kvm.c | 1 - > hw/intc/arm_gicv3_kvm.c | 1 - > 2 files changed, 2 deletions(-) > > diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c > index 6f467e6..204369d 100644 > --- a/hw/intc/arm_gic_kvm.c > +++ b/hw/intc/arm_gic_kvm.c > @@ -572,7 +572,6 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp) > > if (kvm_has_gsi_routing()) { > /* set up irq routing */ > - kvm_init_irq_routing(kvm_state); > for (i = 0; i < s->num_irq - GIC_INTERNAL; ++i) { > kvm_irqchip_add_irq_route(kvm_state, i, 0, i); > } > diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c > index 001d82b..aa4c7c5 100644 > --- a/hw/intc/arm_gicv3_kvm.c > +++ b/hw/intc/arm_gicv3_kvm.c > @@ -813,7 +813,6 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) > > if (kvm_has_gsi_routing()) { > /* set up irq routing */ > - kvm_init_irq_routing(kvm_state); > for (i = 0; i < s->num_irq - GIC_INTERNAL; ++i) { > kvm_irqchip_add_irq_route(kvm_state, i, 0, i); > } >
On 2018/5/31 15:54, Auger Eric wrote: > Hi Shannon, > > On 05/31/2018 09:16 AM, Shannon Zhao wrote: >> kvm_irqchip_create called by kvm_init will call kvm_init_irq_routing to >> initialize global capability variables. If we call kvm_init_irq_routing in >> GIC realize function, previous allocated memory will leak. >> >> Fix this by deleting the unnecessary call. >> >> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> > openpic_kvm seems to suffer the same leak. Don't you want to fix it as > well? > I have a look at below patch of openpic_kvm which says on ppc it doesn't call kvm_irqchip_create. So no such issue for it. commit d85937e683f6ff4d68293cb24c780fb1f6820d2c Author: Scott Wood <scottwood@freescale.com> Date: Wed Jun 12 15:32:51 2013 -0500 kvm/openpic: in-kernel mpic support > Reviewed-by: Eric Auger <eric.auger@redhat.com> > Thanks. > Thanks > > Eric >> --- >> hw/intc/arm_gic_kvm.c | 1 - >> hw/intc/arm_gicv3_kvm.c | 1 - >> 2 files changed, 2 deletions(-) >> >> diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c >> index 6f467e6..204369d 100644 >> --- a/hw/intc/arm_gic_kvm.c >> +++ b/hw/intc/arm_gic_kvm.c >> @@ -572,7 +572,6 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp) >> >> if (kvm_has_gsi_routing()) { >> /* set up irq routing */ >> - kvm_init_irq_routing(kvm_state); >> for (i = 0; i < s->num_irq - GIC_INTERNAL; ++i) { >> kvm_irqchip_add_irq_route(kvm_state, i, 0, i); >> } >> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c >> index 001d82b..aa4c7c5 100644 >> --- a/hw/intc/arm_gicv3_kvm.c >> +++ b/hw/intc/arm_gicv3_kvm.c >> @@ -813,7 +813,6 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) >> >> if (kvm_has_gsi_routing()) { >> /* set up irq routing */ >> - kvm_init_irq_routing(kvm_state); >> for (i = 0; i < s->num_irq - GIC_INTERNAL; ++i) { >> kvm_irqchip_add_irq_route(kvm_state, i, 0, i); >> } >> > > . >
Hi Shannon, On 05/31/2018 10:04 AM, Shannon Zhao wrote: > > > On 2018/5/31 15:54, Auger Eric wrote: >> Hi Shannon, >> >> On 05/31/2018 09:16 AM, Shannon Zhao wrote: >>> kvm_irqchip_create called by kvm_init will call kvm_init_irq_routing to >>> initialize global capability variables. If we call kvm_init_irq_routing in >>> GIC realize function, previous allocated memory will leak. >>> >>> Fix this by deleting the unnecessary call. >>> >>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> >> openpic_kvm seems to suffer the same leak. Don't you want to fix it as >> well? >> > I have a look at below patch of openpic_kvm which says on ppc it doesn't > call kvm_irqchip_create. So no such issue for it. > > commit d85937e683f6ff4d68293cb24c780fb1f6820d2c > Author: Scott Wood <scottwood@freescale.com> > Date: Wed Jun 12 15:32:51 2013 -0500 > > kvm/openpic: in-kernel mpic support Ah OK. Thanks for the pointer. Eric > >> Reviewed-by: Eric Auger <eric.auger@redhat.com> >> > Thanks. > >> Thanks >> >> Eric >>> --- >>> hw/intc/arm_gic_kvm.c | 1 - >>> hw/intc/arm_gicv3_kvm.c | 1 - >>> 2 files changed, 2 deletions(-) >>> >>> diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c >>> index 6f467e6..204369d 100644 >>> --- a/hw/intc/arm_gic_kvm.c >>> +++ b/hw/intc/arm_gic_kvm.c >>> @@ -572,7 +572,6 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp) >>> >>> if (kvm_has_gsi_routing()) { >>> /* set up irq routing */ >>> - kvm_init_irq_routing(kvm_state); >>> for (i = 0; i < s->num_irq - GIC_INTERNAL; ++i) { >>> kvm_irqchip_add_irq_route(kvm_state, i, 0, i); >>> } >>> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c >>> index 001d82b..aa4c7c5 100644 >>> --- a/hw/intc/arm_gicv3_kvm.c >>> +++ b/hw/intc/arm_gicv3_kvm.c >>> @@ -813,7 +813,6 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) >>> >>> if (kvm_has_gsi_routing()) { >>> /* set up irq routing */ >>> - kvm_init_irq_routing(kvm_state); >>> for (i = 0; i < s->num_irq - GIC_INTERNAL; ++i) { >>> kvm_irqchip_add_irq_route(kvm_state, i, 0, i); >>> } >>> >> >> . >> >
On 31 May 2018 at 08:16, Shannon Zhao <zhaoshenglong@huawei.com> wrote: > kvm_irqchip_create called by kvm_init will call kvm_init_irq_routing to > initialize global capability variables. If we call kvm_init_irq_routing in > GIC realize function, previous allocated memory will leak. > > Fix this by deleting the unnecessary call. > > Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> Applied to target-arm.next, thanks. -- PMM
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c index 6f467e6..204369d 100644 --- a/hw/intc/arm_gic_kvm.c +++ b/hw/intc/arm_gic_kvm.c @@ -572,7 +572,6 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp) if (kvm_has_gsi_routing()) { /* set up irq routing */ - kvm_init_irq_routing(kvm_state); for (i = 0; i < s->num_irq - GIC_INTERNAL; ++i) { kvm_irqchip_add_irq_route(kvm_state, i, 0, i); } diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index 001d82b..aa4c7c5 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -813,7 +813,6 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) if (kvm_has_gsi_routing()) { /* set up irq routing */ - kvm_init_irq_routing(kvm_state); for (i = 0; i < s->num_irq - GIC_INTERNAL; ++i) { kvm_irqchip_add_irq_route(kvm_state, i, 0, i); }
kvm_irqchip_create called by kvm_init will call kvm_init_irq_routing to initialize global capability variables. If we call kvm_init_irq_routing in GIC realize function, previous allocated memory will leak. Fix this by deleting the unnecessary call. Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> --- hw/intc/arm_gic_kvm.c | 1 - hw/intc/arm_gicv3_kvm.c | 1 - 2 files changed, 2 deletions(-)