diff mbox series

[v3,12/19] hw/i386/pc: Remove pc_compat_2_7[] array

Message ID 20250502185652.67370-13-philmd@linaro.org
State Superseded
Headers show
Series hw/i386/pc: Remove deprecated 2.6 and 2.7 PC machines | expand

Commit Message

Philippe Mathieu-Daudé May 2, 2025, 6:56 p.m. UTC 
The pc_compat_2_7[] array was only used by the pc-q35-2.7
and pc-i440fx-2.7 machines, which got removed. Remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
 include/hw/i386/pc.h |  3 ---
 hw/i386/pc.c         | 10 ----------
 2 files changed, 13 deletions(-)

Comments

Thomas Huth May 8, 2025, 7:55 a.m. UTC | #1 
On 02/05/2025 20.56, Philippe Mathieu-Daudé wrote:
> The pc_compat_2_7[] array was only used by the pc-q35-2.7
> and pc-i440fx-2.7 machines, which got removed. Remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> ---
>   include/hw/i386/pc.h |  3 ---
>   hw/i386/pc.c         | 10 ----------
>   2 files changed, 13 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 4fb2033bc54..319ec82f709 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -289,9 +289,6 @@ extern const size_t pc_compat_2_9_len;
>   extern GlobalProperty pc_compat_2_8[];
>   extern const size_t pc_compat_2_8_len;
>   
> -extern GlobalProperty pc_compat_2_7[];
> -extern const size_t pc_compat_2_7_len;
> -
>   #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
>       static void pc_machine_##suffix##_class_init(ObjectClass *oc, \
>                                                    const void *data) \
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 7573b880905..ee7095c89a8 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -241,16 +241,6 @@ GlobalProperty pc_compat_2_8[] = {
>   };
>   const size_t pc_compat_2_8_len = G_N_ELEMENTS(pc_compat_2_8);
>   
> -GlobalProperty pc_compat_2_7[] = {
> -    { TYPE_X86_CPU, "l3-cache", "off" },
> -    { TYPE_X86_CPU, "full-cpuid-auto-level", "off" },
> -    { "Opteron_G3" "-" TYPE_X86_CPU, "family", "15" },
> -    { "Opteron_G3" "-" TYPE_X86_CPU, "model", "6" },
> -    { "Opteron_G3" "-" TYPE_X86_CPU, "stepping", "1" },
> -    { "isa-pcspk", "migrate", "off" },
> -};
> -const size_t pc_compat_2_7_len = G_N_ELEMENTS(pc_compat_2_7);

I'd really appreciate if you could provide clean-up patches for 
TYPE_X86_CPU, too. Otherwise I'm pretty sure we'll forget that there is some 
clean up possibility here.

Anyway, for this patch here:
Reviewed-by: Thomas Huth <thuth@redhat.com>
Philippe Mathieu-Daudé May 8, 2025, 10:40 a.m. UTC | #2 
Hi Thomas,

On 8/5/25 09:55, Thomas Huth wrote:
> On 02/05/2025 20.56, Philippe Mathieu-Daudé wrote:
>> The pc_compat_2_7[] array was only used by the pc-q35-2.7
>> and pc-i440fx-2.7 machines, which got removed. Remove it.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
>> ---
>>   include/hw/i386/pc.h |  3 ---
>>   hw/i386/pc.c         | 10 ----------
>>   2 files changed, 13 deletions(-)
>>
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 4fb2033bc54..319ec82f709 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -289,9 +289,6 @@ extern const size_t pc_compat_2_9_len;
>>   extern GlobalProperty pc_compat_2_8[];
>>   extern const size_t pc_compat_2_8_len;
>> -extern GlobalProperty pc_compat_2_7[];
>> -extern const size_t pc_compat_2_7_len;
>> -
>>   #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
>>       static void pc_machine_##suffix##_class_init(ObjectClass *oc, \
>>                                                    const void *data) \
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 7573b880905..ee7095c89a8 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -241,16 +241,6 @@ GlobalProperty pc_compat_2_8[] = {
>>   };
>>   const size_t pc_compat_2_8_len = G_N_ELEMENTS(pc_compat_2_8);
>> -GlobalProperty pc_compat_2_7[] = {
>> -    { TYPE_X86_CPU, "l3-cache", "off" },
>> -    { TYPE_X86_CPU, "full-cpuid-auto-level", "off" },
>> -    { "Opteron_G3" "-" TYPE_X86_CPU, "family", "15" },
>> -    { "Opteron_G3" "-" TYPE_X86_CPU, "model", "6" },
>> -    { "Opteron_G3" "-" TYPE_X86_CPU, "stepping", "1" },
>> -    { "isa-pcspk", "migrate", "off" },
>> -};
>> -const size_t pc_compat_2_7_len = G_N_ELEMENTS(pc_compat_2_7);
> 
> I'd really appreciate if you could provide clean-up patches for 
> TYPE_X86_CPU, too. Otherwise I'm pretty sure we'll forget that there is 
> some clean up possibility here.

Well TBH it is too exhausting to keep rebasing these patches without
feedback from maintainers. I'll respin a v4 with Zhao and your comments
addressed but without touching the TYPE_X86_CPU properties. If
maintainers prefer to remove dead code in one go -- something I
certainly understand from a maintainer PoV -- I'll let someone else
do it, taking over my series.

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

Thank you for your support with these series.

Regards,

Phil.
Thomas Huth May 8, 2025, 11:41 a.m. UTC | #3 
On 08/05/2025 12.40, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 8/5/25 09:55, Thomas Huth wrote:
>> On 02/05/2025 20.56, Philippe Mathieu-Daudé wrote:
>>> The pc_compat_2_7[] array was only used by the pc-q35-2.7
>>> and pc-i440fx-2.7 machines, which got removed. Remove it.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Reviewed-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
>>> ---
>>>   include/hw/i386/pc.h |  3 ---
>>>   hw/i386/pc.c         | 10 ----------
>>>   2 files changed, 13 deletions(-)
>>>
>>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>>> index 4fb2033bc54..319ec82f709 100644
>>> --- a/include/hw/i386/pc.h
>>> +++ b/include/hw/i386/pc.h
>>> @@ -289,9 +289,6 @@ extern const size_t pc_compat_2_9_len;
>>>   extern GlobalProperty pc_compat_2_8[];
>>>   extern const size_t pc_compat_2_8_len;
>>> -extern GlobalProperty pc_compat_2_7[];
>>> -extern const size_t pc_compat_2_7_len;
>>> -
>>>   #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
>>>       static void pc_machine_##suffix##_class_init(ObjectClass *oc, \
>>>                                                    const void *data) \
>>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>>> index 7573b880905..ee7095c89a8 100644
>>> --- a/hw/i386/pc.c
>>> +++ b/hw/i386/pc.c
>>> @@ -241,16 +241,6 @@ GlobalProperty pc_compat_2_8[] = {
>>>   };
>>>   const size_t pc_compat_2_8_len = G_N_ELEMENTS(pc_compat_2_8);
>>> -GlobalProperty pc_compat_2_7[] = {
>>> -    { TYPE_X86_CPU, "l3-cache", "off" },
>>> -    { TYPE_X86_CPU, "full-cpuid-auto-level", "off" },
>>> -    { "Opteron_G3" "-" TYPE_X86_CPU, "family", "15" },
>>> -    { "Opteron_G3" "-" TYPE_X86_CPU, "model", "6" },
>>> -    { "Opteron_G3" "-" TYPE_X86_CPU, "stepping", "1" },
>>> -    { "isa-pcspk", "migrate", "off" },
>>> -};
>>> -const size_t pc_compat_2_7_len = G_N_ELEMENTS(pc_compat_2_7);
>>
>> I'd really appreciate if you could provide clean-up patches for 
>> TYPE_X86_CPU, too. Otherwise I'm pretty sure we'll forget that there is 
>> some clean up possibility here.
> 
> Well TBH it is too exhausting to keep rebasing these patches without
> feedback from maintainers.

Ok, fair, if it's too much right now, it can of course also be done by 
someone else later.

> I'll respin a v4 with Zhao and your comments
> addressed but without touching the TYPE_X86_CPU properties. If
> maintainers prefer to remove dead code in one go -- something I
> certainly understand from a maintainer PoV -- I'll let someone else
> do it, taking over my series.

Since the x86 maintainers seem to be busy with other stuff right now ... I'm 
currently preparing a machine type cleanup pull request that contains 
Daniels patches to automatically disable the old machine types, and my own 
patches to remove the old s390x machine types. If nobody objects, I could 
also add your patches (at least the reviewed ones) there. Michael? Paolo?

  Thomas
Zhao Liu May 8, 2025, 12:09 p.m. UTC | #4 
On Thu, May 08, 2025 at 12:40:35PM +0200, Philippe Mathieu-Daudé wrote:
> Date: Thu, 8 May 2025 12:40:35 +0200
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: Re: [PATCH v3 12/19] hw/i386/pc: Remove pc_compat_2_7[] array
> 
> Hi Thomas,
> 
> On 8/5/25 09:55, Thomas Huth wrote:
> > On 02/05/2025 20.56, Philippe Mathieu-Daudé wrote:
> > > The pc_compat_2_7[] array was only used by the pc-q35-2.7
> > > and pc-i440fx-2.7 machines, which got removed. Remove it.
> > > 
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > Reviewed-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> > > ---
> > >   include/hw/i386/pc.h |  3 ---
> > >   hw/i386/pc.c         | 10 ----------
> > >   2 files changed, 13 deletions(-)
> > > 
> > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > > index 4fb2033bc54..319ec82f709 100644
> > > --- a/include/hw/i386/pc.h
> > > +++ b/include/hw/i386/pc.h
> > > @@ -289,9 +289,6 @@ extern const size_t pc_compat_2_9_len;
> > >   extern GlobalProperty pc_compat_2_8[];
> > >   extern const size_t pc_compat_2_8_len;
> > > -extern GlobalProperty pc_compat_2_7[];
> > > -extern const size_t pc_compat_2_7_len;
> > > -
> > >   #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
> > >       static void pc_machine_##suffix##_class_init(ObjectClass *oc, \
> > >                                                    const void *data) \
> > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > index 7573b880905..ee7095c89a8 100644
> > > --- a/hw/i386/pc.c
> > > +++ b/hw/i386/pc.c
> > > @@ -241,16 +241,6 @@ GlobalProperty pc_compat_2_8[] = {
> > >   };
> > >   const size_t pc_compat_2_8_len = G_N_ELEMENTS(pc_compat_2_8);
> > > -GlobalProperty pc_compat_2_7[] = {
> > > -    { TYPE_X86_CPU, "l3-cache", "off" },
> > > -    { TYPE_X86_CPU, "full-cpuid-auto-level", "off" },
> > > -    { "Opteron_G3" "-" TYPE_X86_CPU, "family", "15" },
> > > -    { "Opteron_G3" "-" TYPE_X86_CPU, "model", "6" },
> > > -    { "Opteron_G3" "-" TYPE_X86_CPU, "stepping", "1" },
> > > -    { "isa-pcspk", "migrate", "off" },
> > > -};
> > > -const size_t pc_compat_2_7_len = G_N_ELEMENTS(pc_compat_2_7);
> > 
> > I'd really appreciate if you could provide clean-up patches for
> > TYPE_X86_CPU, too. Otherwise I'm pretty sure we'll forget that there is
> > some clean up possibility here.
> 
> Well TBH it is too exhausting to keep rebasing these patches without
> feedback from maintainers. I'll respin a v4 with Zhao and your comments
> addressed but without touching the TYPE_X86_CPU properties. If
> maintainers prefer to remove dead code in one go -- something I
> certainly understand from a maintainer PoV -- I'll let someone else
> do it, taking over my series.

Hi Philippe, I think I could volunteer help you to revisit the history
of these properties (they're also too old for me :-)), and help identify
if these properties should be removed or at least list the potential
issues. Hopefully I can do this.

Thanks,
Zhao
diff mbox series

Patch

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 4fb2033bc54..319ec82f709 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -289,9 +289,6 @@  extern const size_t pc_compat_2_9_len;
 extern GlobalProperty pc_compat_2_8[];
 extern const size_t pc_compat_2_8_len;
 
-extern GlobalProperty pc_compat_2_7[];
-extern const size_t pc_compat_2_7_len;
-
 #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
     static void pc_machine_##suffix##_class_init(ObjectClass *oc, \
                                                  const void *data) \
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7573b880905..ee7095c89a8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -241,16 +241,6 @@  GlobalProperty pc_compat_2_8[] = {
 };
 const size_t pc_compat_2_8_len = G_N_ELEMENTS(pc_compat_2_8);
 
-GlobalProperty pc_compat_2_7[] = {
-    { TYPE_X86_CPU, "l3-cache", "off" },
-    { TYPE_X86_CPU, "full-cpuid-auto-level", "off" },
-    { "Opteron_G3" "-" TYPE_X86_CPU, "family", "15" },
-    { "Opteron_G3" "-" TYPE_X86_CPU, "model", "6" },
-    { "Opteron_G3" "-" TYPE_X86_CPU, "stepping", "1" },
-    { "isa-pcspk", "migrate", "off" },
-};
-const size_t pc_compat_2_7_len = G_N_ELEMENTS(pc_compat_2_7);
-
 /*
  * @PC_FW_DATA:
  * Size of the chunk of memory at the top of RAM for the BIOS ACPI tables