[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v6 0/3] Add max-ram-below-4g (was Add pci_hole_min_size machine option)



On 06/17/14 14:41, Michael S. Tsirkin wrote:
On Tue, Jun 17, 2014 at 02:32:07PM -0400, Don Slutz wrote:
Changes v5 to v6:
   rebased on git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git pci

   Changed default handling.  Most pc machines are now set to 3.5G
   (0xe0000000) and q35 are set to 2.75G (0xb0000000).  The special
   value of 0 is used to flag the complex gigabyte_align memory
   layout selection.

   I did change the default for pc-i440fx-2.1 to 3G and pc-q35-2.1 to
   2G instead of the complex gigabyte_align.  This looks ok to me
   because the statement by Gerd Hoffmann is about more ram for 32
   bit (+non-PAE) guests can now have more then the defualt by
   specifing the new option like "max-ram-below-4g=3.75G" or
   "max-ram-below-4g=3.9375G".  Or if that is too complex to
   understand just select pc-i440fx-2.0 or pc-q35-2.0 to get what
   they use to get.
I don't think it's OK.

The beauty of Gerd's hack is that it does the right thing
for 32 bit guests without any need for tweaks.
I.e. most users get good performance.

By comparison this setting seems easier to get wrong than right.

And we definitely don't want to push people to use 2.0
compat setting unless they need bug for bug compatible hardware.

Ok.  Will switch back on a v7.

Generally, I'm still confused about this feature.
It seems to be about helping 32 bit non PAE guests, is that right?

No.  That is just part of it.

But if that's the case why use >4G memory?


I know of a use case where the guest code (custom OS) wants 1G below
4G and 3G above 4G. And 1G below and 7G above.  They want a big MMIO
space for many PCI cards but also still gigabyte aligned.


It seems to me to be better to add the machine option (that is more flexible)
then 4 new pc machine types that have this memory layout.

   -Don Slutz

   Michael S. Tsirkin, Igor Mammedov, Marcel Apfelbaum:
     #2 "pc & q35: Add new machine opt max-ram-below-4g":
        Added setting of .default_machine_opts to include max-ram-below-4g
          for all pc types.
        Removed gigabyte_align.
        Added warning on small value.
        "less then" to "less than"

   #3 "xen-hvm: Pass is_default to xen_hvm_init":
     Changed to work with the changes in #2:
       Added max_ram_below_4g_changed in order to know if it is a default value
       or a user specified one.
     Added "assert(pc_machine->max_ram_below_4g_changed > 0)" so that
       "make check" will abort on default not set for any of the pc or
       q35 machine types.
     Dropped "Acked-by: Stefano Stabellin" do to bigger change.


Changes v4 to v5:
   Re-work based on:

   https://github.com/imammedo/qemu/commits/memory-hotplug-v11

   And so it now depends on this patch set.

   Stefano Stabellini:
     #3 "xen-hvm: Pass is_default to xen_hvm_init"
       Acked-by
       Minor change of pmc to pcms.

Changes v3 to v4:
   Split out #2 "GlobalProperty: Display warning about unused -global"
   rebase on e00fcfe (origin/master)
   rename xen-all to xen-hvm

   Adjust #1 "xen-hvm: Fix xen_hvm_init() to adjust pc memory layout"
     Switch Acked-by & Signed-off-by
     rebase on master

   Rework #3 "xen-hvm: Pass is_default to xen_hvm_init":
     To pass is_default instead of max_ram_below_4g.
     Also did not add "Acked-by: Stefano Stabellini" since code changed a lot.

   Andreas Färber:
     all: Remove dot at end of subject
     #3 "xen-hvm: Pass is_default to xen_hvm_init"
       Adjust comment formatting.

   Andreas Färber, Paolo Bonzini, Marcel Apfelbaum:
     rework to use "opts per machine"
     Drop old #3, new #2.
Changes v2 to v3:
   Stefano Stabellini:
     Acked-by #1 "xen-all: Fix xen_hvm_init() to adjust pc memory"
     Adjust for code readability #4 "xen-all: Pass max_ram_below_4g to 
xen_hvm_init."
        Set max_ram_below_4g always and use it to calculate above_4g_mem_size,
        below_4g_mem_size.

Changes v1 to v2:
   Michael S. Tsirkin:
     Rename option.
     Only add it to machine types that support it.
   Split into 4 parts.

1/4 -- xen-all: Fix xen_hvm_init() to adjust pc memory layout

   This looks to be a possible bug that has yet to be found.
   below_4g_mem_size and above_4g_mem_size are stored in PcGuestInfo
   (pc_guest_info_init) which are currently not "correct".  This and
   4/4 change the same lines.

2/4 -- GlobalProperty: Display warning about unused -global

     My testing showed that setting a global property on an object
     that is not used is not reported at all.  This is added to help
     when the new global is set but not used.  The negative not_used
     was picked so that all static objects are assumed to be used
     even when they are not.

3/4 -- pc & q35: Add new object pc-memory-layout

   The objects that it might make sense to add this property to all
   get created too late.  So add a new object just to hold this
   property.  Name it so that it is expected that only pc (and q35)
   machine types support it.

4/4 -- xen-all: Pass max_ram_below_4g to xen_hvm_init

   Seprate the xen only part of the change.  Currectly based on patch 1/4

Don Slutz (3):
   xen-hvm: Fix xen_hvm_init() to adjust pc memory layout
   pc & q35: Add new machine opt max-ram-below-4g
   xen-hvm: Pass is_default to xen_hvm_init

  hw/i386/pc.c         | 45 ++++++++++++++++++++++++++++++++
  hw/i386/pc_piix.c    | 73 +++++++++++++++++++++++++++++++++-------------------
  hw/i386/pc_q35.c     | 72 ++++++++++++++++++++++++++++++++-------------------
  include/hw/i386/pc.h |  4 +++
  include/hw/xen/xen.h |  3 ++-
  vl.c                 |  4 +++
  xen-hvm-stub.c       |  3 ++-
  xen-hvm.c            | 46 +++++++++++++++++++--------------
  8 files changed, 177 insertions(+), 73 deletions(-)

--
1.8.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.