[v2] microvm: do not use the lastest cpu version

[PATCH v2] microvm: do not use the lastest cpu version

Posted by Ani Sinha 11 months, 3 weeks ago

commit 0788a56bd1ae3 ("i386: Make unversioned CPU models be aliases")
introduced 'default_cpu_version' for PCMachineClass. This created three
categories of CPU models:
 - Most unversioned CPU models would use version 1 by default.
 - For machines 4.0.1 and older that do not support cpu model aliases, a
   special default_cpu_version value of CPU_VERSION_LEGACY is used.
 - It was thought that future machines would use the latest value of cpu
   versions corresponding to default_cpu_version value of
   CPU_VERSION_LATEST [1].

All pc machines still use the default cpu version of 1 for
unversioned cpu models. CPU_VERSION_LATEST is a moving target and
changes with time. Therefore, if machines use CPU_VERSION_LATEST, it would
mean that over a period of time, for the same versioned machine type,
the cpu version would be different depending on what the latest was at that
time. This would break guests even when they use a constant specific
versioned machine type.
Additionally, microvm machines are not versioned anyway and therefore
there is no requirement to use the latest cpu model by default.
Let microvms use the non-versioned cpu model and remove all references
to CPU_VERSION_LATEST as there are no other users (nor we anticipate
future consumers of CPU_VERSION_LATEST).

Those users who need spefific cpu versions can use explicit version in
the QEMU command line to select the specific cpu version desired.

CI pipline does not break with this change.

1) See commit dcafd1ef0af227 ("i386: Register versioned CPU models")

CC: imammedo@redhat.com
CC: zhao1.liu@intel.com
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
 hw/i386/microvm.c |  2 +-
 target/i386/cpu.c | 15 ---------------
 target/i386/cpu.h |  4 ----
 3 files changed, 1 insertion(+), 20 deletions(-)

changelog:
v2: tags added, more explanation in the commit log.

diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index a8d354aabe..b8be1542ff 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -458,7 +458,7 @@ static void microvm_machine_state_init(MachineState *machine)
 
     microvm_memory_init(mms);
 
-    x86_cpus_init(x86ms, CPU_VERSION_LATEST);
+    x86_cpus_init(x86ms, 1);
 
     microvm_devices_init(mms);
 }
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 72ab147e85..0a95b33d15 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5513,18 +5513,6 @@ void x86_cpu_set_default_version(X86CPUVersion version)
     default_cpu_version = version;
 }
 
-static X86CPUVersion x86_cpu_model_last_version(const X86CPUModel *model)
-{
-    int v = 0;
-    const X86CPUVersionDefinition *vdef =
-        x86_cpu_def_get_versions(model->cpudef);
-    while (vdef->version) {
-        v = vdef->version;
-        vdef++;
-    }
-    return v;
-}
-
 /* Return the actual version being used for a specific CPU model */
 static X86CPUVersion x86_cpu_model_resolve_version(const X86CPUModel *model)
 {
@@ -5532,9 +5520,6 @@ static X86CPUVersion x86_cpu_model_resolve_version(const X86CPUModel *model)
     if (v == CPU_VERSION_AUTO) {
         v = default_cpu_version;
     }
-    if (v == CPU_VERSION_LATEST) {
-        return x86_cpu_model_last_version(model);
-    }
     return v;
 }
 
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index c67b42d34f..71f150a05f 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2701,10 +2701,6 @@ void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
                                    TPRAccess access);
 
 /* Special values for X86CPUVersion: */
-
-/* Resolve to latest CPU version */
-#define CPU_VERSION_LATEST -1
-
 /*
  * Resolve to version defined by current machine type.
  * See x86_cpu_set_default_version()
-- 
2.45.2

Re: [PATCH v2] microvm: do not use the lastest cpu version

Posted by Paolo Bonzini 10 months, 3 weeks ago

On 2/20/25 07:53, Ani Sinha wrote:
> commit 0788a56bd1ae3 ("i386: Make unversioned CPU models be aliases")
> introduced 'default_cpu_version' for PCMachineClass. This created three
> categories of CPU models:
>   - Most unversioned CPU models would use version 1 by default.
>   - For machines 4.0.1 and older that do not support cpu model aliases, a
>     special default_cpu_version value of CPU_VERSION_LEGACY is used.
>   - It was thought that future machines would use the latest value of cpu
>     versions corresponding to default_cpu_version value of
>     CPU_VERSION_LATEST [1].
> 
> All pc machines still use the default cpu version of 1 for
> unversioned cpu models. CPU_VERSION_LATEST is a moving target and
> changes with time.

Personally I believe this is a problem and I'd rather use 
CPU_VERSION_LATEST for the unversioned pc and q35 models, just like 
microvm does.

Unversioned models change the hardware properties and there's no reason 
for CPU properties to be treated differently.  Unversioned models are 
exactly for when you are okay with a moving target.

And independent of this, microvm could start having versioned variants, 
so that pc and q35 work the same way.

Paolo

Re: [PATCH v2] microvm: do not use the lastest cpu version

Posted by Michael S. Tsirkin 10 months, 3 weeks ago

On Tue, Mar 18, 2025 at 12:01:44PM +0100, Paolo Bonzini wrote:
> On 2/20/25 07:53, Ani Sinha wrote:
> > commit 0788a56bd1ae3 ("i386: Make unversioned CPU models be aliases")
> > introduced 'default_cpu_version' for PCMachineClass. This created three
> > categories of CPU models:
> >   - Most unversioned CPU models would use version 1 by default.
> >   - For machines 4.0.1 and older that do not support cpu model aliases, a
> >     special default_cpu_version value of CPU_VERSION_LEGACY is used.
> >   - It was thought that future machines would use the latest value of cpu
> >     versions corresponding to default_cpu_version value of
> >     CPU_VERSION_LATEST [1].
> > 
> > All pc machines still use the default cpu version of 1 for
> > unversioned cpu models. CPU_VERSION_LATEST is a moving target and
> > changes with time.
> 
> Personally I believe this is a problem and I'd rather use CPU_VERSION_LATEST
> for the unversioned pc and q35 models, just like microvm does.

I don't object.

> Unversioned models change the hardware properties and there's no reason for
> CPU properties to be treated differently.  Unversioned models are exactly
> for when you are okay with a moving target.
> 
> And independent of this, microvm could start having versioned variants, so
> that pc and q35 work the same way.
> 
> Paolo
>

Re: [PATCH v2] microvm: do not use the lastest cpu version

Posted by Zhao Liu 11 months, 3 weeks ago

On Thu, Feb 20, 2025 at 12:23:26PM +0530, Ani Sinha wrote:
> Date: Thu, 20 Feb 2025 12:23:26 +0530
> From: Ani Sinha <anisinha@redhat.com>
> Subject: [PATCH v2] microvm: do not use the lastest cpu version
> X-Mailer: git-send-email 2.45.2
> 
> commit 0788a56bd1ae3 ("i386: Make unversioned CPU models be aliases")
> introduced 'default_cpu_version' for PCMachineClass. This created three
> categories of CPU models:
>  - Most unversioned CPU models would use version 1 by default.
>  - For machines 4.0.1 and older that do not support cpu model aliases, a
>    special default_cpu_version value of CPU_VERSION_LEGACY is used.
>  - It was thought that future machines would use the latest value of cpu
>    versions corresponding to default_cpu_version value of
>    CPU_VERSION_LATEST [1].
> 
> All pc machines still use the default cpu version of 1 for
> unversioned cpu models. CPU_VERSION_LATEST is a moving target and
> changes with time. Therefore, if machines use CPU_VERSION_LATEST, it would
> mean that over a period of time, for the same versioned machine type,
> the cpu version would be different depending on what the latest was at that
> time. This would break guests even when they use a constant specific
> versioned machine type.
> Additionally, microvm machines are not versioned anyway and therefore
> there is no requirement to use the latest cpu model by default.
> Let microvms use the non-versioned cpu model and remove all references
> to CPU_VERSION_LATEST as there are no other users (nor we anticipate
> future consumers of CPU_VERSION_LATEST).
> 
> Those users who need spefific cpu versions can use explicit version in
> the QEMU command line to select the specific cpu version desired.
> 
> CI pipline does not break with this change.
> 
> 1) See commit dcafd1ef0af227 ("i386: Register versioned CPU models")
> 
> CC: imammedo@redhat.com
> CC: zhao1.liu@intel.com
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Sergio Lopez <slp@redhat.com>
> Signed-off-by: Ani Sinha <anisinha@redhat.com>
> ---
>  hw/i386/microvm.c |  2 +-
>  target/i386/cpu.c | 15 ---------------
>  target/i386/cpu.h |  4 ----
>  3 files changed, 1 insertion(+), 20 deletions(-)
> 
> changelog:
> v2: tags added, more explanation in the commit log.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH v2] microvm: do not use the lastest cpu version

Posted by Ani Sinha 11 months, 1 week ago

On Thu, Feb 20, 2025 at 12:36 PM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> On Thu, Feb 20, 2025 at 12:23:26PM +0530, Ani Sinha wrote:
> > Date: Thu, 20 Feb 2025 12:23:26 +0530
> > From: Ani Sinha <anisinha@redhat.com>
> > Subject: [PATCH v2] microvm: do not use the lastest cpu version
> > X-Mailer: git-send-email 2.45.2
> >
> > commit 0788a56bd1ae3 ("i386: Make unversioned CPU models be aliases")
> > introduced 'default_cpu_version' for PCMachineClass. This created three
> > categories of CPU models:
> >  - Most unversioned CPU models would use version 1 by default.
> >  - For machines 4.0.1 and older that do not support cpu model aliases, a
> >    special default_cpu_version value of CPU_VERSION_LEGACY is used.
> >  - It was thought that future machines would use the latest value of cpu
> >    versions corresponding to default_cpu_version value of
> >    CPU_VERSION_LATEST [1].
> >
> > All pc machines still use the default cpu version of 1 for
> > unversioned cpu models. CPU_VERSION_LATEST is a moving target and
> > changes with time. Therefore, if machines use CPU_VERSION_LATEST, it would
> > mean that over a period of time, for the same versioned machine type,
> > the cpu version would be different depending on what the latest was at that
> > time. This would break guests even when they use a constant specific
> > versioned machine type.
> > Additionally, microvm machines are not versioned anyway and therefore
> > there is no requirement to use the latest cpu model by default.
> > Let microvms use the non-versioned cpu model and remove all references
> > to CPU_VERSION_LATEST as there are no other users (nor we anticipate
> > future consumers of CPU_VERSION_LATEST).
> >
> > Those users who need spefific cpu versions can use explicit version in
> > the QEMU command line to select the specific cpu version desired.
> >
> > CI pipline does not break with this change.
> >
> > 1) See commit dcafd1ef0af227 ("i386: Register versioned CPU models")
> >
> > CC: imammedo@redhat.com
> > CC: zhao1.liu@intel.com
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > Reviewed-by: Sergio Lopez <slp@redhat.com>
> > Signed-off-by: Ani Sinha <anisinha@redhat.com>
> > ---
> >  hw/i386/microvm.c |  2 +-
> >  target/i386/cpu.c | 15 ---------------
> >  target/i386/cpu.h |  4 ----
> >  3 files changed, 1 insertion(+), 20 deletions(-)
> >
> > changelog:
> > v2: tags added, more explanation in the commit log.
>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>

Who is picking this up?

Re: [PATCH v2] microvm: do not use the lastest cpu version

Posted by Ani Sinha 11 months, 1 week ago

On Sat, Mar 1, 2025 at 9:04 PM Ani Sinha <anisinha@redhat.com> wrote:
>
> On Thu, Feb 20, 2025 at 12:36 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> >
> > On Thu, Feb 20, 2025 at 12:23:26PM +0530, Ani Sinha wrote:
> > > Date: Thu, 20 Feb 2025 12:23:26 +0530
> > > From: Ani Sinha <anisinha@redhat.com>
> > > Subject: [PATCH v2] microvm: do not use the lastest cpu version
> > > X-Mailer: git-send-email 2.45.2
> > >
> > > commit 0788a56bd1ae3 ("i386: Make unversioned CPU models be aliases")
> > > introduced 'default_cpu_version' for PCMachineClass. This created three
> > > categories of CPU models:
> > >  - Most unversioned CPU models would use version 1 by default.
> > >  - For machines 4.0.1 and older that do not support cpu model aliases, a
> > >    special default_cpu_version value of CPU_VERSION_LEGACY is used.
> > >  - It was thought that future machines would use the latest value of cpu
> > >    versions corresponding to default_cpu_version value of
> > >    CPU_VERSION_LATEST [1].
> > >
> > > All pc machines still use the default cpu version of 1 for
> > > unversioned cpu models. CPU_VERSION_LATEST is a moving target and
> > > changes with time. Therefore, if machines use CPU_VERSION_LATEST, it would
> > > mean that over a period of time, for the same versioned machine type,
> > > the cpu version would be different depending on what the latest was at that
> > > time. This would break guests even when they use a constant specific
> > > versioned machine type.
> > > Additionally, microvm machines are not versioned anyway and therefore
> > > there is no requirement to use the latest cpu model by default.
> > > Let microvms use the non-versioned cpu model and remove all references
> > > to CPU_VERSION_LATEST as there are no other users (nor we anticipate
> > > future consumers of CPU_VERSION_LATEST).
> > >
> > > Those users who need spefific cpu versions can use explicit version in
> > > the QEMU command line to select the specific cpu version desired.
> > >
> > > CI pipline does not break with this change.
> > >
> > > 1) See commit dcafd1ef0af227 ("i386: Register versioned CPU models")
> > >
> > > CC: imammedo@redhat.com
> > > CC: zhao1.liu@intel.com
> > > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > > Reviewed-by: Sergio Lopez <slp@redhat.com>
> > > Signed-off-by: Ani Sinha <anisinha@redhat.com>
> > > ---
> > >  hw/i386/microvm.c |  2 +-
> > >  target/i386/cpu.c | 15 ---------------
> > >  target/i386/cpu.h |  4 ----
> > >  3 files changed, 1 insertion(+), 20 deletions(-)
> > >
> > > changelog:
> > > v2: tags added, more explanation in the commit log.
> >
> > Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> >
>
> Who is picking this up?

I sent a pull request for this and a couple other reviewed patches
myself. Two reasons:
- wanted to see this in the upstream sooner as some other bits of the
work is pending on it.
- I never sent a pull request before and wanted to go through the
process to learn how to do it in case I needed it in the future.

i hope the PR is ok. If not, I can resend after corrections. I used
Peter's script https://git.linaro.org/people/peter.maydell/misc-scripts.git/plain/make-pullreq

Re: [PATCH v2] microvm: do not use the lastest cpu version

Posted by Stefan Hajnoczi 11 months, 1 week ago

On Wed, Mar 05, 2025 at 01:24:25PM +0530, Ani Sinha wrote:
> On Sat, Mar 1, 2025 at 9:04 PM Ani Sinha <anisinha@redhat.com> wrote:
> >
> > On Thu, Feb 20, 2025 at 12:36 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> > >
> > > On Thu, Feb 20, 2025 at 12:23:26PM +0530, Ani Sinha wrote:
> > > > Date: Thu, 20 Feb 2025 12:23:26 +0530
> > > > From: Ani Sinha <anisinha@redhat.com>
> > > > Subject: [PATCH v2] microvm: do not use the lastest cpu version
> > > > X-Mailer: git-send-email 2.45.2
> > > >
> > > > commit 0788a56bd1ae3 ("i386: Make unversioned CPU models be aliases")
> > > > introduced 'default_cpu_version' for PCMachineClass. This created three
> > > > categories of CPU models:
> > > >  - Most unversioned CPU models would use version 1 by default.
> > > >  - For machines 4.0.1 and older that do not support cpu model aliases, a
> > > >    special default_cpu_version value of CPU_VERSION_LEGACY is used.
> > > >  - It was thought that future machines would use the latest value of cpu
> > > >    versions corresponding to default_cpu_version value of
> > > >    CPU_VERSION_LATEST [1].
> > > >
> > > > All pc machines still use the default cpu version of 1 for
> > > > unversioned cpu models. CPU_VERSION_LATEST is a moving target and
> > > > changes with time. Therefore, if machines use CPU_VERSION_LATEST, it would
> > > > mean that over a period of time, for the same versioned machine type,
> > > > the cpu version would be different depending on what the latest was at that
> > > > time. This would break guests even when they use a constant specific
> > > > versioned machine type.
> > > > Additionally, microvm machines are not versioned anyway and therefore
> > > > there is no requirement to use the latest cpu model by default.
> > > > Let microvms use the non-versioned cpu model and remove all references
> > > > to CPU_VERSION_LATEST as there are no other users (nor we anticipate
> > > > future consumers of CPU_VERSION_LATEST).
> > > >
> > > > Those users who need spefific cpu versions can use explicit version in
> > > > the QEMU command line to select the specific cpu version desired.
> > > >
> > > > CI pipline does not break with this change.
> > > >
> > > > 1) See commit dcafd1ef0af227 ("i386: Register versioned CPU models")
> > > >
> > > > CC: imammedo@redhat.com
> > > > CC: zhao1.liu@intel.com
> > > > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > > > Reviewed-by: Sergio Lopez <slp@redhat.com>
> > > > Signed-off-by: Ani Sinha <anisinha@redhat.com>
> > > > ---
> > > >  hw/i386/microvm.c |  2 +-
> > > >  target/i386/cpu.c | 15 ---------------
> > > >  target/i386/cpu.h |  4 ----
> > > >  3 files changed, 1 insertion(+), 20 deletions(-)
> > > >
> > > > changelog:
> > > > v2: tags added, more explanation in the commit log.
> > >
> > > Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> > >
> >
> > Who is picking this up?
> 
> I sent a pull request for this and a couple other reviewed patches
> myself. Two reasons:
> - wanted to see this in the upstream sooner as some other bits of the
> work is pending on it.
> - I never sent a pull request before and wanted to go through the
> process to learn how to do it in case I needed it in the future.
> 
> i hope the PR is ok. If not, I can resend after corrections. I used
> Peter's script https://git.linaro.org/people/peter.maydell/misc-scripts.git/plain/make-pullreq

This should go via Paolo's tree. I have pinged him to remind him of your
patches.

Please only send pull requests for subsystems where you are listed as
the maintainer in the MAINTAINERS file.

It doesn't scale when people send me PRs directly because I need to do a
bunch of extra sanity checking and helping people get their one-off PRs
properly signed and formatted. I also don't like to bypass
sub-maintainers because I'm less qualified to do the final review than
the sub-maintainers themselves.

Thanks,
Stefan

Re: [PATCH v2] microvm: do not use the lastest cpu version

Posted by Ani Sinha 11 months, 1 week ago

On Wed, Mar 5, 2025 at 7:12 PM Stefan Hajnoczi <stefanha@gmail.com> wrote:
>
> On Wed, Mar 05, 2025 at 01:24:25PM +0530, Ani Sinha wrote:
> > On Sat, Mar 1, 2025 at 9:04 PM Ani Sinha <anisinha@redhat.com> wrote:
> > >
> > > On Thu, Feb 20, 2025 at 12:36 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> > > >
> > > > On Thu, Feb 20, 2025 at 12:23:26PM +0530, Ani Sinha wrote:
> > > > > Date: Thu, 20 Feb 2025 12:23:26 +0530
> > > > > From: Ani Sinha <anisinha@redhat.com>
> > > > > Subject: [PATCH v2] microvm: do not use the lastest cpu version
> > > > > X-Mailer: git-send-email 2.45.2
> > > > >
> > > > > commit 0788a56bd1ae3 ("i386: Make unversioned CPU models be aliases")
> > > > > introduced 'default_cpu_version' for PCMachineClass. This created three
> > > > > categories of CPU models:
> > > > >  - Most unversioned CPU models would use version 1 by default.
> > > > >  - For machines 4.0.1 and older that do not support cpu model aliases, a
> > > > >    special default_cpu_version value of CPU_VERSION_LEGACY is used.
> > > > >  - It was thought that future machines would use the latest value of cpu
> > > > >    versions corresponding to default_cpu_version value of
> > > > >    CPU_VERSION_LATEST [1].
> > > > >
> > > > > All pc machines still use the default cpu version of 1 for
> > > > > unversioned cpu models. CPU_VERSION_LATEST is a moving target and
> > > > > changes with time. Therefore, if machines use CPU_VERSION_LATEST, it would
> > > > > mean that over a period of time, for the same versioned machine type,
> > > > > the cpu version would be different depending on what the latest was at that
> > > > > time. This would break guests even when they use a constant specific
> > > > > versioned machine type.
> > > > > Additionally, microvm machines are not versioned anyway and therefore
> > > > > there is no requirement to use the latest cpu model by default.
> > > > > Let microvms use the non-versioned cpu model and remove all references
> > > > > to CPU_VERSION_LATEST as there are no other users (nor we anticipate
> > > > > future consumers of CPU_VERSION_LATEST).
> > > > >
> > > > > Those users who need spefific cpu versions can use explicit version in
> > > > > the QEMU command line to select the specific cpu version desired.
> > > > >
> > > > > CI pipline does not break with this change.
> > > > >
> > > > > 1) See commit dcafd1ef0af227 ("i386: Register versioned CPU models")
> > > > >
> > > > > CC: imammedo@redhat.com
> > > > > CC: zhao1.liu@intel.com
> > > > > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > > > > Reviewed-by: Sergio Lopez <slp@redhat.com>
> > > > > Signed-off-by: Ani Sinha <anisinha@redhat.com>
> > > > > ---
> > > > >  hw/i386/microvm.c |  2 +-
> > > > >  target/i386/cpu.c | 15 ---------------
> > > > >  target/i386/cpu.h |  4 ----
> > > > >  3 files changed, 1 insertion(+), 20 deletions(-)
> > > > >
> > > > > changelog:
> > > > > v2: tags added, more explanation in the commit log.
> > > >
> > > > Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> > > >
> > >
> > > Who is picking this up?
> >
> > I sent a pull request for this and a couple other reviewed patches
> > myself. Two reasons:
> > - wanted to see this in the upstream sooner as some other bits of the
> > work is pending on it.
> > - I never sent a pull request before and wanted to go through the
> > process to learn how to do it in case I needed it in the future.
> >
> > i hope the PR is ok. If not, I can resend after corrections. I used
> > Peter's script https://git.linaro.org/people/peter.maydell/misc-scripts.git/plain/make-pullreq
>
> This should go via Paolo's tree. I have pinged him to remind him of your
> patches.
>
> Please only send pull requests for subsystems where you are listed as
> the maintainer in the MAINTAINERS file.
>
> It doesn't scale when people send me PRs directly because I need to do a
> bunch of extra sanity checking and helping people get their one-off PRs
> properly signed and formatted. I also don't like to bypass
> sub-maintainers because I'm less qualified to do the final review than
> the sub-maintainers themselves.

Fair enough! I hope the three patches get merged soon.

Re: [PATCH v2] microvm: do not use the lastest cpu version

Posted by Ani Sinha 10 months, 3 weeks ago

On Wed, Mar 5, 2025 at 7:56 PM Ani Sinha <anisinha@redhat.com> wrote:
>
> On Wed, Mar 5, 2025 at 7:12 PM Stefan Hajnoczi <stefanha@gmail.com> wrote:
> >
> > On Wed, Mar 05, 2025 at 01:24:25PM +0530, Ani Sinha wrote:
> > > On Sat, Mar 1, 2025 at 9:04 PM Ani Sinha <anisinha@redhat.com> wrote:
> > > >
> > > > On Thu, Feb 20, 2025 at 12:36 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> > > > >
> > > > > On Thu, Feb 20, 2025 at 12:23:26PM +0530, Ani Sinha wrote:
> > > > > > Date: Thu, 20 Feb 2025 12:23:26 +0530
> > > > > > From: Ani Sinha <anisinha@redhat.com>
> > > > > > Subject: [PATCH v2] microvm: do not use the lastest cpu version
> > > > > > X-Mailer: git-send-email 2.45.2
> > > > > >
> > > > > > commit 0788a56bd1ae3 ("i386: Make unversioned CPU models be aliases")
> > > > > > introduced 'default_cpu_version' for PCMachineClass. This created three
> > > > > > categories of CPU models:
> > > > > >  - Most unversioned CPU models would use version 1 by default.
> > > > > >  - For machines 4.0.1 and older that do not support cpu model aliases, a
> > > > > >    special default_cpu_version value of CPU_VERSION_LEGACY is used.
> > > > > >  - It was thought that future machines would use the latest value of cpu
> > > > > >    versions corresponding to default_cpu_version value of
> > > > > >    CPU_VERSION_LATEST [1].
> > > > > >
> > > > > > All pc machines still use the default cpu version of 1 for
> > > > > > unversioned cpu models. CPU_VERSION_LATEST is a moving target and
> > > > > > changes with time. Therefore, if machines use CPU_VERSION_LATEST, it would
> > > > > > mean that over a period of time, for the same versioned machine type,
> > > > > > the cpu version would be different depending on what the latest was at that
> > > > > > time. This would break guests even when they use a constant specific
> > > > > > versioned machine type.
> > > > > > Additionally, microvm machines are not versioned anyway and therefore
> > > > > > there is no requirement to use the latest cpu model by default.
> > > > > > Let microvms use the non-versioned cpu model and remove all references
> > > > > > to CPU_VERSION_LATEST as there are no other users (nor we anticipate
> > > > > > future consumers of CPU_VERSION_LATEST).
> > > > > >
> > > > > > Those users who need spefific cpu versions can use explicit version in
> > > > > > the QEMU command line to select the specific cpu version desired.
> > > > > >
> > > > > > CI pipline does not break with this change.
> > > > > >
> > > > > > 1) See commit dcafd1ef0af227 ("i386: Register versioned CPU models")
> > > > > >
> > > > > > CC: imammedo@redhat.com
> > > > > > CC: zhao1.liu@intel.com
> > > > > > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > > > > > Reviewed-by: Sergio Lopez <slp@redhat.com>
> > > > > > Signed-off-by: Ani Sinha <anisinha@redhat.com>
> > > > > > ---
> > > > > >  hw/i386/microvm.c |  2 +-
> > > > > >  target/i386/cpu.c | 15 ---------------
> > > > > >  target/i386/cpu.h |  4 ----
> > > > > >  3 files changed, 1 insertion(+), 20 deletions(-)
> > > > > >
> > > > > > changelog:
> > > > > > v2: tags added, more explanation in the commit log.
> > > > >
> > > > > Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> > > > >
> > > >
> > > > Who is picking this up?
> > >
> > > I sent a pull request for this and a couple other reviewed patches
> > > myself. Two reasons:
> > > - wanted to see this in the upstream sooner as some other bits of the
> > > work is pending on it.
> > > - I never sent a pull request before and wanted to go through the
> > > process to learn how to do it in case I needed it in the future.
> > >
> > > i hope the PR is ok. If not, I can resend after corrections. I used
> > > Peter's script https://git.linaro.org/people/peter.maydell/misc-scripts.git/plain/make-pullreq
> >
> > This should go via Paolo's tree. I have pinged him to remind him of your
> > patches.
> >
> > Please only send pull requests for subsystems where you are listed as
> > the maintainer in the MAINTAINERS file.
> >
> > It doesn't scale when people send me PRs directly because I need to do a
> > bunch of extra sanity checking and helping people get their one-off PRs
> > properly signed and formatted. I also don't like to bypass
> > sub-maintainers because I'm less qualified to do the final review than
> > the sub-maintainers themselves.
>
> Fair enough! I hope the three patches get merged soon.

This has still not merged! Unfortunate.