Re: [PATCH v3] target/i386: Fix CPUID encoding of Fn8000001E_ECX

[Moger, Babu]

Hi Daniel,

On 5/10/2024 3:10 AM, Daniel P. Berrangé wrote:
> On Fri, May 10, 2024 at 11:05:44AM +0300, Michael Tokarev wrote:
> > 09.05.2024 17:11, Daniel P. Berrangé wrote:
> > > On Thu, May 09, 2024 at 04:54:16PM +0300, Michael Tokarev wrote:
> > > > 03.05.2024 20:46, Babu Moger wrote:
> >
> > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > > > index 08c7de416f..46235466d7 100644
> > > > > --- a/hw/i386/pc.c
> > > > > +++ b/hw/i386/pc.c
> > > > > @@ -81,6 +81,7 @@
> > > > >     GlobalProperty pc_compat_9_0[] = {
> > > > >         { TYPE_X86_CPU, "guest-phys-bits", "0" },
> > > > >         { "sev-guest", "legacy-vm-type", "true" },
> > > > > +    { TYPE_X86_CPU, "legacy-multi-node", "on" },
> > > > >     };
> > > >
> > > > Should this legacy-multi-node property be added to previous
> > > > machine types when applying to stable?  How about stable-8.2
> > > > and stable-7.2?
> > >
> > > machine types are considered to express a fixed guest ABI
> > > once part of a QEMU release. Given that we should not be
> > > changing existing machine types in stable branches.
> >
> > Yes, I understand this, and this is exactly why I asked.
> > The change in question has been Cc'ed to stable.  And I'm
> > trying to understand what should I do with it :)
> >
> > > In theory we could create new "bug fix" machine types in stable
> > > branches. To support live migration, we would then need to also
> > > add those same stable branch "bug fix" machine type versions in
> > > all future QEMU versions. This is generally not worth the hassle
> > > of exploding the number of machine types.
> > >
> > > If you backport the patch, minus the machine type, then users
> > > can still get the fix but they'll need to manually set the
> > > property to enable it.
> >
> > I don't think this makes big sense.  But maybe for someone who
> > actually hits this issue such backport will let to fix it.
> > Hence, again, I'm asking if it really a good idea to pick this
> > up for stable (any version of, - currently there are 2 active
> > series, 7.2, 8.2 and 9.0).
>
> Hmm, the description says
>
>    "Observed the following failure while booting the SEV-SNP guest"
>
> and yet the patches for SEV-SNP are *not* merged in QEMU yet. So this
> does not look relevant for stable unless I'm missing something.

I have not thought thru about stable tag. This is not critical for 
stable release.

If required I will send a separate patch for stable later. It is not 
required right now. Sorry about the noise.

--
- Babu Moger