[PATCH] pci/shpc: Do not unparent in instance_finalize()

Akihiko Odaki posted 1 patch 3 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251027-shpc-v1-1-00e9b20a355d@rsg.ci.i.u-tokyo.ac.jp
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
hw/pci/shpc.c | 1 -
1 file changed, 1 deletion(-)
[PATCH] pci/shpc: Do not unparent in instance_finalize()
Posted by Akihiko Odaki 3 months, 1 week ago
Children are automatically unparented so manually unparenting is
unnecessary.

Worse, automatic unparenting happens before the instance_finalize()
callback of the parent gets called, so object_unparent() calls in
the callback will refer to objects that are already unparented, which
is semantically incorrect.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
See also:
https://lore.kernel.org/qemu-devel/20250924-use-v4-0-07c6c598f53d@rsg.ci.i.u-tokyo.ac.jp
---
 hw/pci/shpc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index aac6f2d03459..938602866d77 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -735,7 +735,6 @@ void shpc_free(PCIDevice *d)
     if (!shpc) {
         return;
     }
-    object_unparent(OBJECT(&shpc->mmio));
     g_free(shpc->config);
     g_free(shpc->cmask);
     g_free(shpc->wmask);

---
base-commit: c85ba2d7a4056595166689890285105579db446a
change-id: 20251027-shpc-d71bb72d67af

Best regards,
--  
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Re: [PATCH] pci/shpc: Do not unparent in instance_finalize()
Posted by Michael Tokarev 1 day, 7 hours ago
On 10/27/25 04:24, Akihiko Odaki wrote:
> Children are automatically unparented so manually unparenting is
> unnecessary.
> 
> Worse, automatic unparenting happens before the instance_finalize()
> callback of the parent gets called, so object_unparent() calls in
> the callback will refer to objects that are already unparented, which
> is semantically incorrect.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

Is this a qemu-stable material?

Please let me know if it is -- I'm *not* queuing it up for now.

Thanks,

/mjt

> ---
> See also:
> https://lore.kernel.org/qemu-devel/20250924-use-v4-0-07c6c598f53d@rsg.ci.i.u-tokyo.ac.jp
> ---
>   hw/pci/shpc.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
> index aac6f2d03459..938602866d77 100644
> --- a/hw/pci/shpc.c
> +++ b/hw/pci/shpc.c
> @@ -735,7 +735,6 @@ void shpc_free(PCIDevice *d)
>       if (!shpc) {
>           return;
>       }
> -    object_unparent(OBJECT(&shpc->mmio));
>       g_free(shpc->config);
>       g_free(shpc->cmask);
>       g_free(shpc->wmask);
> 
> ---
> base-commit: c85ba2d7a4056595166689890285105579db446a
> change-id: 20251027-shpc-d71bb72d67af
> 
> Best regards,
> --
> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> 
> 
>
Re: [PATCH] pci/shpc: Do not unparent in instance_finalize()
Posted by Akihiko Odaki an hour ago
On 2026/02/06 6:25, Michael Tokarev wrote:
> On 10/27/25 04:24, Akihiko Odaki wrote:
>> Children are automatically unparented so manually unparenting is
>> unnecessary.
>>
>> Worse, automatic unparenting happens before the instance_finalize()
>> callback of the parent gets called, so object_unparent() calls in
>> the callback will refer to objects that are already unparented, which
>> is semantically incorrect.
>>
>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> 
> Is this a qemu-stable material?

I don't think so. It's semantically incorrect but I don't see a 
possibility that it causes a real problem. Cherry-picking it shouldn't 
cause a harm though. Having this patch or not shouldn't make any 
behavioral change.

Regards,
Akihiko Odaki

> 
> Please let me know if it is -- I'm *not* queuing it up for now.
> 
> Thanks,
> 
> /mjt
> 
>> ---
>> See also:
>> https://lore.kernel.org/qemu-devel/20250924-use- 
>> v4-0-07c6c598f53d@rsg.ci.i.u-tokyo.ac.jp
>> ---
>>   hw/pci/shpc.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
>> index aac6f2d03459..938602866d77 100644
>> --- a/hw/pci/shpc.c
>> +++ b/hw/pci/shpc.c
>> @@ -735,7 +735,6 @@ void shpc_free(PCIDevice *d)
>>       if (!shpc) {
>>           return;
>>       }
>> -    object_unparent(OBJECT(&shpc->mmio));
>>       g_free(shpc->config);
>>       g_free(shpc->cmask);
>>       g_free(shpc->wmask);
>>
>> ---
>> base-commit: c85ba2d7a4056595166689890285105579db446a
>> change-id: 20251027-shpc-d71bb72d67af
>>
>> Best regards,
>> -- 
>> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>
>>
>>
>