From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49688) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WwyVf-0000Mr-2C for qemu-devel@nongnu.org; Tue, 17 Jun 2014 14:53:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WwyVY-0006sk-HX for qemu-devel@nongnu.org; Tue, 17 Jun 2014 14:53:31 -0400 Received: from fldsmtpe04.verizon.com ([140.108.26.143]:49485) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WwyVY-0006sa-9e for qemu-devel@nongnu.org; Tue, 17 Jun 2014 14:53:24 -0400 From: Don Slutz Message-ID: <53A08EA1.90603@terremark.com> Date: Tue, 17 Jun 2014 14:53:21 -0400 MIME-Version: 1.0 References: <1403029930-28358-1-git-send-email-dslutz@verizon.com> <20140617184115.GA15610@redhat.com> In-Reply-To: <20140617184115.GA15610@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v6 0/3] Add max-ram-below-4g (was Add pci_hole_min_size machine option) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , Don Slutz Cc: xen-devel@lists.xensource.com, Stefano Stabellini , Marcel Apfelbaum , qemu-devel@nongnu.org, Anthony Liguori , Igor Mammedov , =?ISO-8859-1?Q?Andreas_F=E4rber?= 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [PATCH v6 0/3] Add max-ram-below-4g (was Add pci_hole_min_size machine option) Date: Tue, 17 Jun 2014 14:53:21 -0400 Message-ID: <53A08EA1.90603@terremark.com> References: <1403029930-28358-1-git-send-email-dslutz@verizon.com> <20140617184115.GA15610@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: <20140617184115.GA15610@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org To: "Michael S. Tsirkin" , Don Slutz Cc: xen-devel@lists.xensource.com, Stefano Stabellini , Marcel Apfelbaum , qemu-devel@nongnu.org, Anthony Liguori , Igor Mammedov , =?ISO-8859-1?Q?Andreas_F=E4rber?= List-Id: xen-devel@lists.xenproject.org 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