qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: "Daniel P. Berrange" <berrange@redhat.com>
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, ehabkost@redhat.com,
	marcel@redhat.com, mst@redhat.com, kraxel@redhat.com
Subject: Re: [Qemu-devel] [PATCH v2 5/6] x86: fix up 32 bit phys_bits case
Date: Tue, 5 Jul 2016 20:05:54 +0100	[thread overview]
Message-ID: <20160705190553.GB7695@work-vm> (raw)
In-Reply-To: <20160705115543.GK6553@redhat.com>

* Daniel P. Berrange (berrange@redhat.com) wrote:
> On Tue, Jul 05, 2016 at 12:29:30PM +0100, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrange (berrange@redhat.com) wrote:
> > > On Mon, Jul 04, 2016 at 08:16:08PM +0100, Dr. David Alan Gilbert (git) wrote:
> > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > > 
> > > > On 32 bit systems fix up phys_bits to be consistent with what
> > > > we tell the guest; don't ever bother with using the phys_bits
> > > > property.
> > > 
> > > > @@ -2990,6 +2986,15 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> > > >              cpu->phys_bits = host_phys_bits;
> > > >  
> > > >          }
> > > > +    } else {
> > > > +        /* For 32 bit systems don't use the user set value, but keep
> > > > +         * phys_bits consistent with what we tell the guest.
> > > > +         */
> > > > +        if (env->features[FEAT_1_EDX] & CPUID_PSE36) {
> > > > +            cpu->phys_bits = 36;
> > > > +        } else {
> > > > +            cpu->phys_bits = 32;
> > > > +        }
> > > 
> > > I kind of feel like we should report an error and exit if the
> > > user/app has provided a phys_bits property value, rather than
> > > silently ignoring their provided value, on the basis that this
> > > is a user/app configuration error.
> > 
> > Do we have an easy way to tell that the user has set the parameter
> > as opposed to it being the default?
> 
> Not sure if there's an official way, but you could perhaps default
> phys_bits to -1, and treat -1 as being equivalent to 0 if set by
> the user.

Done in v3 (I used 9999 since I'd kept it as an unsigned)

Dave

> 
> 
> Regards,
> Daniel
> -- 
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

  reply	other threads:[~2016-07-05 19:06 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-04 19:16 [Qemu-devel] [PATCH v2 0/6] x86: Physical address limit patches Dr. David Alan Gilbert (git)
2016-07-04 19:16 ` [Qemu-devel] [PATCH v2 1/6] x86: Allow physical address bits to be set Dr. David Alan Gilbert (git)
2016-07-04 19:33   ` Eduardo Habkost
2016-07-05 13:43     ` Dr. David Alan Gilbert
2016-07-04 19:16 ` [Qemu-devel] [PATCH v2 2/6] x86: Mask mtrr mask based on CPU physical address limits Dr. David Alan Gilbert (git)
2016-07-04 20:02   ` Michael S. Tsirkin
2016-07-04 20:05     ` Eduardo Habkost
2016-07-04 22:37       ` Michael S. Tsirkin
2016-07-04 20:03   ` Eduardo Habkost
2016-07-04 19:16 ` [Qemu-devel] [PATCH v2 3/6] x86: fill high bits of mtrr mask Dr. David Alan Gilbert (git)
2016-07-04 20:03   ` Michael S. Tsirkin
2016-07-04 20:14     ` Eduardo Habkost
2016-07-04 20:21   ` Eduardo Habkost
2016-07-05  8:39     ` Dr. David Alan Gilbert
2016-07-04 19:16 ` [Qemu-devel] [PATCH v2 4/6] x86: Set physical address bits based on host Dr. David Alan Gilbert (git)
2016-07-04 20:27   ` Eduardo Habkost
2016-07-05  8:44     ` Dr. David Alan Gilbert
2016-07-04 19:16 ` [Qemu-devel] [PATCH v2 5/6] x86: fix up 32 bit phys_bits case Dr. David Alan Gilbert (git)
2016-07-05  9:42   ` Daniel P. Berrange
2016-07-05 11:29     ` Dr. David Alan Gilbert
2016-07-05 11:55       ` Daniel P. Berrange
2016-07-05 19:05         ` Dr. David Alan Gilbert [this message]
2016-07-04 19:16 ` [Qemu-devel] [PATCH v2 6/6] x86: Add sanity checks on phys_bits Dr. David Alan Gilbert (git)
2016-07-04 20:46   ` Eduardo Habkost
2016-07-05 10:40     ` Dr. David Alan Gilbert
2016-07-04 20:23 ` [Qemu-devel] [PATCH v2 0/6] x86: Physical address limit patches Michael S. Tsirkin
2016-07-05  9:33   ` Dr. David Alan Gilbert
2016-07-05 10:06     ` Michael S. Tsirkin
2016-07-05 10:13       ` Dr. David Alan Gilbert
2016-07-05 10:41         ` Michael S. Tsirkin
2016-07-05 10:59       ` Paolo Bonzini
2016-07-05 11:09         ` Michael S. Tsirkin
2016-07-05 11:46           ` Paolo Bonzini
2016-07-05 12:39             ` Michael S. Tsirkin
2016-07-05 12:41           ` Dr. David Alan Gilbert
2016-07-05 13:38             ` Michael S. Tsirkin
2016-07-05  9:46 ` Daniel P. Berrange
2016-07-05  9:49   ` Dr. David Alan Gilbert
2016-07-05 12:38     ` Eduardo Habkost

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160705190553.GB7695@work-vm \
    --to=dgilbert@redhat.com \
    --cc=berrange@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=marcel@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).