Re: [Qemu-devel] [PATCH v3 1/4] x86: Allow physical address bits to be set

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Eduardo Habkost (ehabkost@redhat.com) wrote:
> On Thu, Jul 07, 2016 at 05:39:14PM +0100, Dr. David Alan Gilbert wrote:
> [...]
> > * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > > On Tue, Jul 05, 2016 at 08:03:15PM +0100, Dr. David Alan Gilbert (git) wrote:
> > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > > 
> > > > Currently QEMU sets the x86 number of physical address bits to the
> > > > magic number 40.  This is only correct on some small AMD systems;
> > > > Intel systems tend to have 36, 39, 46 bits, and large AMD systems
> > > > tend to have 48.
> > > > 
> > > > Having the value different from your actual hardware is detectable
> > > > by the guest and in principal can cause problems;
> > > > The current limit of 40 stops TB VMs being created by those lucky
> > > > enough to have that much.
> > > > 
> > > > This patch lets you set the physical bits by a cpu property but
> > > > defaults to the same existing magic 40.
> > > > 
> > > > I've removed the ancient warning about the 42 bit limit in exec.c;
> > > > I can't find that limit in there and no one else seems to know where
> > > > it is.
> > > > 
> > > > We use a magic value of 9999 as the property default so that we can
> > > > later distinguish between the default and a user set value.
> > > 
> > > I'd prefer to use -1 or 0xFFFFFFFF (UINT32_MAX) to indicate it
> > > was not set by the user, and not document it as "use the old
> > > default" but just as "it was not set explicitly".
> > > 
> > > This won't allow us to differentiate between "set by user" and
> > > "set by machine-type compat_props" in the future. But I would
> > > prefer to use a MachineClass field or boolean property than magic
> > > numbers for that, anyway.
> > > 
> > > If you replace 9999 with UINT32_MAX or 0xFFFFFFFF in this patch:
> > > 
> > > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> > > 
> > > BTW, using 0 to indicate "not set" would be acceptable, too, but
> > > the magic 0 value in patch 4/4 would need to be replaced with a
> > > boolean "host-phys-bits" property. But I prefer boolean properties
> > > instead of magic numbers, anyway.
> > 
> > OK, lets do that then.  I'll use 0 here for the magic default
> > and then add the host-phys-bits boolean that strictly overrides
> > the phys-bits numeric.
> > 
> > I didn't use UINT32_MAX or 0xffffffff because I wasn't convinced
> > how that would interact with the machine-type/compat code which
> > uses strings to represent default values for machine types.
> 
> Good point. Integer parsing code in QEMU is ... interesting.
> 
> > 
> > (And we have ~40 cases of DEFINE_PROP_UINT*(.....,-1) which I just
> > find very wrong).
> 
> Just wait until you see how string-input-visitor.c parses
> uint64_t values.

Blech!

Dave

> 
> -- 
> Eduardo
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK