hw/vfio/ap: attribute constructor for cfg_chg_events_lock

[RFC PATCH v1] hw/vfio/ap: attribute constructor for cfg_chg_events_lock

Posted by Rorie Reyes 5 months ago

Created an attribute constructor for cfg_chg_events_lock for locking
mechanism when storing event information for an AP configuration change
event

Fixes: fd03360215 ("Storing event information for an AP configuration change event")
Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
---
 hw/vfio/ap.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index 874e0d1eaf..1df4438149 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -52,6 +52,11 @@ static QTAILQ_HEAD(, APConfigChgEvent) cfg_chg_events =
 
 static QemuMutex cfg_chg_events_lock;
 
+static void __attribute__((constructor)) vfio_ap_global_init(void)
+{
+    qemu_mutex_init(&cfg_chg_events_lock);
+}
+
 OBJECT_DECLARE_SIMPLE_TYPE(VFIOAPDevice, VFIO_AP_DEVICE)
 
 static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
@@ -230,13 +235,6 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
     VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev);
     VFIODevice *vbasedev = &vapdev->vdev;
 
-    static bool lock_initialized;
-
-    if (!lock_initialized) {
-        qemu_mutex_init(&cfg_chg_events_lock);
-        lock_initialized = true;
-    }
-
     if (!vfio_device_get_name(vbasedev, errp)) {
         return;
     }
-- 
2.48.1

Re: [RFC PATCH v1] hw/vfio/ap: attribute constructor for cfg_chg_events_lock

Posted by Cédric Le Goater 5 months ago

On 6/11/25 23:12, Rorie Reyes wrote:
> Created an attribute constructor for cfg_chg_events_lock for locking
> mechanism when storing event information for an AP configuration change
> event
> 
> Fixes: fd03360215 ("Storing event information for an AP configuration change event")
> Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
> ---
>   hw/vfio/ap.c | 12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index 874e0d1eaf..1df4438149 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -52,6 +52,11 @@ static QTAILQ_HEAD(, APConfigChgEvent) cfg_chg_events =
>   
>   static QemuMutex cfg_chg_events_lock;
>   
> +static void __attribute__((constructor)) vfio_ap_global_init(void)
> +{
> +    qemu_mutex_init(&cfg_chg_events_lock);
> +}
> +
>   OBJECT_DECLARE_SIMPLE_TYPE(VFIOAPDevice, VFIO_AP_DEVICE)
>   
>   static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
> @@ -230,13 +235,6 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
>       VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev);
>       VFIODevice *vbasedev = &vapdev->vdev;
>   
> -    static bool lock_initialized;
> -
> -    if (!lock_initialized) {
> -        qemu_mutex_init(&cfg_chg_events_lock);
> -        lock_initialized = true;
> -    }
> -
>       if (!vfio_device_get_name(vbasedev, errp)) {
>           return;
>       }


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Applied to vfio-next.

Thanks,

C.

Re: [RFC PATCH v1] hw/vfio/ap: attribute constructor for cfg_chg_events_lock

Posted by Rorie Reyes 5 months ago

On 6/12/25 2:18 AM, Cédric Le Goater wrote:
> On 6/11/25 23:12, Rorie Reyes wrote:
>> Created an attribute constructor for cfg_chg_events_lock for locking
>> mechanism when storing event information for an AP configuration change
>> event
>>
>> Fixes: fd03360215 ("Storing event information for an AP configuration 
>> change event")
>> Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
>> ---
>>   hw/vfio/ap.c | 12 +++++-------
>>   1 file changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
>> index 874e0d1eaf..1df4438149 100644
>> --- a/hw/vfio/ap.c
>> +++ b/hw/vfio/ap.c
>> @@ -52,6 +52,11 @@ static QTAILQ_HEAD(, APConfigChgEvent) 
>> cfg_chg_events =
>>     static QemuMutex cfg_chg_events_lock;
>>   +static void __attribute__((constructor)) vfio_ap_global_init(void)
>> +{
>> +    qemu_mutex_init(&cfg_chg_events_lock);
>> +}
>> +
>>   OBJECT_DECLARE_SIMPLE_TYPE(VFIOAPDevice, VFIO_AP_DEVICE)
>>     static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
>> @@ -230,13 +235,6 @@ static void vfio_ap_realize(DeviceState *dev, 
>> Error **errp)
>>       VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev);
>>       VFIODevice *vbasedev = &vapdev->vdev;
>>   -    static bool lock_initialized;
>> -
>> -    if (!lock_initialized) {
>> -        qemu_mutex_init(&cfg_chg_events_lock);
>> -        lock_initialized = true;
>> -    }
>> -
>>       if (!vfio_device_get_name(vbasedev, errp)) {
>>           return;
>>       }
>
>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
>
> Applied to vfio-next.
>
> Thanks,
>
> C.
>
Thank you!

Re: [RFC PATCH v1] hw/vfio/ap: attribute constructor for cfg_chg_events_lock

Posted by Thomas Huth 5 months ago

On 11/06/2025 23.12, Rorie Reyes wrote:
> Created an attribute constructor for cfg_chg_events_lock for locking
> mechanism when storing event information for an AP configuration change
> event
> 
> Fixes: fd03360215 ("Storing event information for an AP configuration change event")
> Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
> ---
>   hw/vfio/ap.c | 12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index 874e0d1eaf..1df4438149 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -52,6 +52,11 @@ static QTAILQ_HEAD(, APConfigChgEvent) cfg_chg_events =
>   
>   static QemuMutex cfg_chg_events_lock;
>   
> +static void __attribute__((constructor)) vfio_ap_global_init(void)
> +{
> +    qemu_mutex_init(&cfg_chg_events_lock);
> +}

I wonder why we don't have a macro for this yet (there are many other spots 
in the code that initialize a mutex like this).

Anyway, for your patch:
Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [RFC PATCH v1] hw/vfio/ap: attribute constructor for cfg_chg_events_lock

Posted by Rorie Reyes 5 months ago

On 6/12/25 12:44 AM, Thomas Huth wrote:
> On 11/06/2025 23.12, Rorie Reyes wrote:
>> Created an attribute constructor for cfg_chg_events_lock for locking
>> mechanism when storing event information for an AP configuration change
>> event
>>
>> Fixes: fd03360215 ("Storing event information for an AP configuration 
>> change event")
>> Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
>> ---
>>   hw/vfio/ap.c | 12 +++++-------
>>   1 file changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
>> index 874e0d1eaf..1df4438149 100644
>> --- a/hw/vfio/ap.c
>> +++ b/hw/vfio/ap.c
>> @@ -52,6 +52,11 @@ static QTAILQ_HEAD(, APConfigChgEvent) 
>> cfg_chg_events =
>>     static QemuMutex cfg_chg_events_lock;
>>   +static void __attribute__((constructor)) vfio_ap_global_init(void)
>> +{
>> +    qemu_mutex_init(&cfg_chg_events_lock);
>> +}
>
> I wonder why we don't have a macro for this yet (there are many other 
> spots in the code that initialize a mutex like this).
>
> Anyway, for your patch:
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
Thank you!