Re: [PATCH v3] x86: add etc/phys-bits fw_cfg file

[Gerd Hoffmann]

On Thu, Sep 22, 2022 at 04:55:16AM -0400, Michael S. Tsirkin wrote:
> On Thu, Sep 22, 2022 at 10:43:56AM +0200, Gerd Hoffmann wrote:
> > In case phys bits are functional and can be used by the guest (aka
> > host-phys-bits=on) add a fw_cfg file carrying the value.  This can
> > be used by the guest firmware for address space configuration.
> > 
> > This is only enabled for 7.2+ machine types for live migration
> > compatibility reasons.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> I'm curious why you decided to switch from a cpuid flag to fw cfg.

The kernel people didn't like the cpuid approach.

> I guess firmware reads fw cfg anyway.

Correct.

> But would the guest kernel then need to load a fw cfg driver very
> early to detect this, too?

Nope, the guest kernel can just work with the address space layout
created by the firmware.  The firmware can for example reserve a
larger 64-bit mmio window in case there is enough address space for
that.  So it programs the bridge windows etc accordingly, qemu
generates matching acpi tables and the kernel picks up the changes
via _CRS.

> > +void fw_cfg_phys_bits(FWCfgState *fw_cfg)
> > +{
> > +    X86CPU *cpu = X86_CPU(first_cpu);
> > +    uint64_t phys_bits = cpu->phys_bits;
> > +
> > +    if (cpu->host_phys_bits) {
> > +        fw_cfg_add_file(fw_cfg, "etc/phys-bits",
> > +                        g_memdup2(&phys_bits, sizeof(phys_bits)),
> > +                        sizeof(phys_bits));
> > +    }
> > +}
> 
> So, this allows a lot of flexibility, any phys_bits value at all can now
> be used. Do you expect a use-case for such a flexible mechanism?  If
> this ends up merely repeating CPUID at all times then we are just
> creating confusion.

Yes, it'll just repeat CPUID.  Advantage is that the guest gets the
information it needs right away.

Alternatively I could create a "etc/reliable-phys-bits" bool.
The firmware must consult both fw_cfg and cpuid then.

take care,
  Gerd