On Wed, Mar 20, 2024 at 3:45 AM Xiaoyao Li <xiaoyao.li@xxxxxxxxx> wrote:
> If users pass configuration like "-cpu
> qemu64,phys-bits=52,host-phys-bits-limit=45", the cpu->guest_phys_bits
> will be set to 45. I think this is not what we want, though the usage
> seems insane.
>
> We can guard it as
>
> if (cpu->host_phys_bits && cpu->host_phys_bits_limit &&
> cpu->guest_phys_bits > cpu->host_phys_bits_limt)
> {
> }
> Simpler, we can guard with cpu->phys_bits like below, because
> cpu->host_phys_bits_limit is used to guard cpu->phys_bits in
> host_cpu_realizefn()
>
> if (cpu->guest_phys_bits > cpu->phys_bits) {
> cpu->guest_phys_bits = cpu->phys_bits;
> }
>
>
> with this resolved,
>
> Reviewed-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxx>
[oops sorry - I noticed now that this email was never sent, so I am
sending it for archival]
There are more issues:
1) for compatibility with older machine types, the GuestPhysAddrSize
should remain 0. One possibility is to have "-1" as "accelerator
default" and "0" as "show it as zero in CPUID".
2) a "guest-phys-bits is not user-configurable in 32 bit" error is
probably a good idea just like it does for cpu->phys_bits
3) I think the order of the patches makes more sense if the property
is added first and KVM is adjusted second.
I'll post a v5 myself (mostly because it has to include the creation
of 9.1 machine types).
Paolo
> > + }
> > +}
> > +
> > static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
> > {
> > X86CPU *cpu = X86_CPU(cs);
> > CPUX86State *env = &cpu->env;
> > + bool ret;
> >
> > /*
> > * The realize order is important, since x86_cpu_realize() checks if
> > @@ -50,7 +73,13 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
> > MSR_IA32_UCODE_REV);
> > }
> > }
> > - return host_cpu_realizefn(cs, errp);
> > + ret = host_cpu_realizefn(cs, errp);
> > + if (!ret) {
> > + return ret;
> > + }
> > +
> > + kvm_set_guest_phys_bits(cs);
> > + return true;
> > }
> >
> > static bool lmce_supported(void)
>Ther
