1. Patchew
  2. QEMU
  3. [PATCH v2 00/16] hw/i386/pc: Remove deprecated 2.6 and 2.7 PC machines
  4. View patch

[PATCH v2 05/16] hw/intc/apic: Remove APICCommonState::legacy_instance_id field

Philippe Mathieu-Daudé posted 16 patches 6 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 05/16] hw/intc/apic: Remove APICCommonState::legacy_instance_id field
Posted by Philippe Mathieu-Daudé 6 months, 2 weeks ago 
The APICCommonState::legacy_instance_id boolean was only set
in the pc_compat_2_6[] array, via the 'legacy-instance-id=on'
property. We removed all machines using that array, lets remove
that property, simplifying apic_common_realize().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/apic_internal.h | 1 -
 hw/intc/apic_common.c           | 5 -----
 2 files changed, 6 deletions(-)

diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 429278da618..db6a9101530 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -188,7 +188,6 @@ struct APICCommonState {
     uint32_t vapic_control;
     DeviceState *vapic;
     hwaddr vapic_paddr; /* note: persistence via kvmvapic */
-    bool legacy_instance_id;
     uint32_t extended_log_dest;
 };
 
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 37a7a7019d3..1d259b97e63 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -294,9 +294,6 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
         info->enable_tpr_reporting(s, true);
     }
 
-    if (s->legacy_instance_id) {
-        instance_id = VMSTATE_INSTANCE_ID_ANY;
-    }
     vmstate_register_with_alias_id(NULL, instance_id, &vmstate_apic_common,
                                    s, -1, 0, NULL);
 
@@ -412,8 +409,6 @@ static const Property apic_properties_common[] = {
     DEFINE_PROP_UINT8("version", APICCommonState, version, 0x14),
     DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
                     true),
-    DEFINE_PROP_BOOL("legacy-instance-id", APICCommonState, legacy_instance_id,
-                     false),
 };
 
 static void apic_common_get_id(Object *obj, Visitor *v, const char *name,
-- 
2.47.1
Re: [PATCH v2 05/16] hw/intc/apic: Remove APICCommonState::legacy_instance_id field
Posted by Thomas Huth 6 months, 2 weeks ago 
On 01/05/2025 20.36, Philippe Mathieu-Daudé wrote:
> The APICCommonState::legacy_instance_id boolean was only set
> in the pc_compat_2_6[] array, via the 'legacy-instance-id=on'
> property. We removed all machines using that array, lets remove
> that property, simplifying apic_common_realize().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/i386/apic_internal.h | 1 -
>   hw/intc/apic_common.c           | 5 -----
>   2 files changed, 6 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>
Re: [PATCH v2 05/16] hw/intc/apic: Remove APICCommonState::legacy_instance_id field
Posted by Mark Cave-Ayland 6 months, 2 weeks ago 
On 01/05/2025 19:36, Philippe Mathieu-Daudé wrote:

> The APICCommonState::legacy_instance_id boolean was only set
> in the pc_compat_2_6[] array, via the 'legacy-instance-id=on'
> property. We removed all machines using that array, lets remove
> that property, simplifying apic_common_realize().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/i386/apic_internal.h | 1 -
>   hw/intc/apic_common.c           | 5 -----
>   2 files changed, 6 deletions(-)
> 
> diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
> index 429278da618..db6a9101530 100644
> --- a/include/hw/i386/apic_internal.h
> +++ b/include/hw/i386/apic_internal.h
> @@ -188,7 +188,6 @@ struct APICCommonState {
>       uint32_t vapic_control;
>       DeviceState *vapic;
>       hwaddr vapic_paddr; /* note: persistence via kvmvapic */
> -    bool legacy_instance_id;
>       uint32_t extended_log_dest;
>   };
>   
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 37a7a7019d3..1d259b97e63 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -294,9 +294,6 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
>           info->enable_tpr_reporting(s, true);
>       }
>   
> -    if (s->legacy_instance_id) {
> -        instance_id = VMSTATE_INSTANCE_ID_ANY;
> -    }
>       vmstate_register_with_alias_id(NULL, instance_id, &vmstate_apic_common,
>                                      s, -1, 0, NULL);

With the legacy_instance_id removed, is it now also possible to register 
vmstate_apic_common directly via dc->vmsd instead?

> @@ -412,8 +409,6 @@ static const Property apic_properties_common[] = {
>       DEFINE_PROP_UINT8("version", APICCommonState, version, 0x14),
>       DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
>                       true),
> -    DEFINE_PROP_BOOL("legacy-instance-id", APICCommonState, legacy_instance_id,
> -                     false),
>   };
>   
>   static void apic_common_get_id(Object *obj, Visitor *v, const char *name,

Anyhow:

Reviewed-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>


ATB,

Mark.
Re: [PATCH v2 05/16] hw/intc/apic: Remove APICCommonState::legacy_instance_id field
Posted by Philippe Mathieu-Daudé 6 months, 2 weeks ago 
Hi Mark,

On 2/5/25 11:14, Mark Cave-Ayland wrote:
> On 01/05/2025 19:36, Philippe Mathieu-Daudé wrote:
> 
>> The APICCommonState::legacy_instance_id boolean was only set
>> in the pc_compat_2_6[] array, via the 'legacy-instance-id=on'
>> property. We removed all machines using that array, lets remove
>> that property, simplifying apic_common_realize().
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/hw/i386/apic_internal.h | 1 -
>>   hw/intc/apic_common.c           | 5 -----
>>   2 files changed, 6 deletions(-)
>>
>> diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/ 
>> apic_internal.h
>> index 429278da618..db6a9101530 100644
>> --- a/include/hw/i386/apic_internal.h
>> +++ b/include/hw/i386/apic_internal.h
>> @@ -188,7 +188,6 @@ struct APICCommonState {
>>       uint32_t vapic_control;
>>       DeviceState *vapic;
>>       hwaddr vapic_paddr; /* note: persistence via kvmvapic */
>> -    bool legacy_instance_id;
>>       uint32_t extended_log_dest;
>>   };
>> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
>> index 37a7a7019d3..1d259b97e63 100644
>> --- a/hw/intc/apic_common.c
>> +++ b/hw/intc/apic_common.c
>> @@ -294,9 +294,6 @@ static void apic_common_realize(DeviceState *dev, 
>> Error **errp)
>>           info->enable_tpr_reporting(s, true);
>>       }
>> -    if (s->legacy_instance_id) {
>> -        instance_id = VMSTATE_INSTANCE_ID_ANY;
>> -    }
>>       vmstate_register_with_alias_id(NULL, instance_id, 
>> &vmstate_apic_common,
>>                                      s, -1, 0, NULL);
> 
> With the legacy_instance_id removed, is it now also possible to register 
> vmstate_apic_common directly via dc->vmsd instead?

I don't think so because it still uses initial_apic_id, but I'm not sure:

     uint32_t instance_id = s->initial_apic_id;

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.