Re: [PATCH 0/2] expose host-phys-bits to guest

[Michael S. Tsirkin]

On Fri, Sep 02, 2022 at 08:07:20AM +0200, Gerd Hoffmann wrote:
> On Fri, Sep 02, 2022 at 08:10:00AM +0800, Xiaoyao Li wrote:
> > On 9/2/2022 12:17 AM, Gerd Hoffmann wrote:
> > > On Thu, Sep 01, 2022 at 10:36:19PM +0800, Xiaoyao Li wrote:
> > > > On 9/1/2022 9:58 PM, Gerd Hoffmann wrote:
> > > > 
> > > > > > Anyway, IMO, guest including guest firmware, should always consult 
> > > > > > from
> > > > > > CPUID leaf 0x80000008 for physical address length.
> > > > > 
> > > > > It simply can't for the reason outlined above.  Even if we fix qemu
> > > > > today that doesn't solve the problem for the firmware because we want
> > > > > backward compatibility with older qemu versions.  Thats why I want the
> > > > > extra bit which essentially says "CPUID leaf 0x80000008 actually 
> > > > > works".
> > > > 
> > > > I don't understand how it backward compatible with older qemu version. 
> > > > Old
> > > > QEMU won't set the extra bit you introduced in this series, and all the
> > > > guest created with old QEMU will become untrusted on CPUID leaf 
> > > > 0x80000008 ?
> > > 
> > > Correct, on old qemu firmware will not trust CPUID leaf 0x80000008.
> > > That is not worse than the situation we have today, currently the
> > > firmware never trusts CPUID leaf 0x80000008.
> > > 
> > > So the patches will improves the situation for new qemu only, but I
> > > don't see a way around that.
> > > 
> > 
> > I see.
> > 
> > But IMHO, I don't think it's good that guest firmware workaround the issue
> > on its own. Instead, it's better to just trust CPUID leaf 0x80000008 and
> > fail if the given physical address length cannot be virtualized/supported.
> > 
> > It's just the bug of VMM to virtualize the physical address length. The
> > correction direction is to fix the bug not the workaround to hide the bug.
> 
> I'm starting to repeat myself. "just trust CPUID leaf 0x80000008"
> doesn't work because you simply can't with current qemu versions.
> 
> I don't like the dance with the new bit very much either, but I don't
> see a better way without massive fallout due to compatibility problems.
> I'm open to suggestions though.
> 
> take care,
>   Gerd


I feel there are three major sources of controversy here

0. the cover letter and subject don't do such a good job
   explaining that what we are doing is just telling guest
   CPUID is not broken. we are not exposing anything new
   and not exposing host capability to guest, for example,
   if cpuid phys address is smaller than host things also
   work fine.

1. really the naming.  We need to be more explicit that it's just a bugfix.

2. down the road we will want to switch the default when no PV. however,
   some hosts might still want conservative firmware for compatibility
   reasons, so I think we need a way to tell firmware
   "ignore phys address width in CPUID like you did in the past".
   let's add a flag for that?
   and if none are set firmware should print a warning, though I
   do not know how many people will see that. Maybe some ;)

along the lines of:

/*
 * Old KVM hosts often reported incorrect phys address width,
 * so firmware had to be very conservative in its use of physical
 * addresses. 
 * One of the two following flags should be set.
 * If none are set firmware is for now conservative, but that will
 * likely change in the future, hosts should not rely on that.
 */
/* 
/* KVM with non broken phys address width should set this flag
 * firmware will be allowed to use all phys address bits
 */
#define KVM_BUG_PHYS_ADDRESS_WIDTH_NONBROKEN 1
/*
 * Force firmware to be very conservative in its use of physical
 * addresses, ignoring phys address width in CPUID.
 * Helpful for migration between hosts with different capabilities.
 */
#define KVM_BUG_PHYS_ADDRESS_WIDTH_BROKEN 2

-- 
MST