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

[Eduardo Habkost]

On Thu, Jul 07, 2016 at 05:39:14PM +0100, Dr. David Alan Gilbert wrote:
[...]
> * Eduardo Habkost (address@hidden) wrote:
> > On Tue, Jul 05, 2016 at 08:03:15PM +0100, Dr. David Alan Gilbert (git) 
> > wrote:
> > > From: "Dr. David Alan Gilbert" <address@hidden>
> > > 
> > > 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 <address@hidden>
> > 
> > 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.

-- 
Eduardo