Re: [PATCH v2 2/2] kvm: add support for guest physical bits

[Paolo Bonzini]

On Mon, Mar 11, 2024 at 12:59 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > > index 952174bb6f52..d427218827f6 100644
> > > --- a/target/i386/cpu.h
> > > +++ b/target/i386/cpu.h
> > > +    guest_phys_bits = kvm_get_guest_phys_bits(cs->kvm_state);
> > > +    if (guest_phys_bits &&
> > > +        (cpu->guest_phys_bits == 0 ||
> > > +         cpu->guest_phys_bits > guest_phys_bits)) {
> > > +        cpu->guest_phys_bits = guest_phys_bits;
> > > +    }
> >
> > Like Xiaoyao mentioned, the right place for this is kvm_cpu_realizefn,
> > after host_cpu_realizefn returns. It should also be conditional on
> > cpu->host_phys_bits.
>
> Ok.
>
> > It also makes sense to:
> >
> > - make kvm_get_guest_phys_bits() return bits 7:0 if bits 23:16 are zero
> >
> > - here, set cpu->guest_phys_bits only if it is not equal to
> > cpu->phys_bits (this undoes the previous suggestion, but I think it's
> > cleaner)
>
> Not sure about that.
>
> I think it would be good to have a backward compatibility story.
> Currently neither the kernel nor qemu set guest_phys_bits.  So if the
> firmware finds guest_phys_bits == 0 it does not know whenever ...
>
>   (a) kernel or qemu being too old, or
>   (b) no restrictions apply, it is safe to go with phys_bits.
>
> One easy option would be to always let qemu pass through guest_phys_bits
> from the kernel, even in case it is equal to phys_bits.

Ah, I see - you would like to be able to use all 52 bits (instead of
going for a safer 46 or 48) and therefore you need to have nonzero
guest_phys_bits even if it's equal to phys_bits. While on an old
kernel, you would pass forward 0.

> > - add a property in x86_cpu_properties[] to allow configuration with TCG.
>
> Was thinking about configuration too.  Not sure it is a good idea to
> add yet another phys-bits config option to the mix of options we already
> have ...

I think it's nice that you can use TCG to test various cases, which
requires a new property.

> In case host_phys_bits=true qemu could simply use
> min(kernel guest-phys-bits,host-phys-bits-limit)

Yes, that works.

Paolo

> For the host_phys_bits=false case it would probably be best to just
> not set guest_phys_bits.