From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [PATCH v3 1/4] xen-all: Fix xen_hvm_init() to adjust pc memory layout. Date: Tue, 22 Apr 2014 12:29:03 -0400 Message-ID: <535698CF.1070702@terremark.com> References: <1395705336-22528-1-git-send-email-dslutz@verizon.com> <1395705336-22528-2-git-send-email-dslutz@verizon.com> <53513553.3040005@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: <53513553.3040005@suse.de> 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: =?ISO-8859-15?Q?Andreas_F=E4rber?= , Don Slutz Cc: xen-devel@lists.xensource.com, Eduardo Habkost , "Michael S. Tsirkin" , Stefano Stabellini , qemu-devel@nongnu.org, Anthony Liguori , Igor Mammedov List-Id: xen-devel@lists.xenproject.org On 04/18/14 10:23, Andreas Färber wrote: > Am 25.03.2014 00:55, schrieb Don Slutz: >> This is just below_4g_mem_size and above_4g_mem_size which is used later in QEMU. >> >> Signed-off-by: Don Slutz >> Acked-by: Stefano Stabellini > Please remember to place your Signed-off-by last. In theory you would > place another Signed-off-by last, but in practice we usually drop the > first one if there are no other Sobs. Think of someone taking your > patch, making changes and applying it to some downstream; then Stefano > did not ack the modified patch signed off by someone else, but rather > you are asserting that Stefano acked the patch in the form you are > sending it. I will try to remember this. Not sure Xen follows the same rules. >> --- >> hw/i386/pc_piix.c | 31 ++++++++++++++++--------------- >> hw/i386/pc_q35.c | 29 +++++++++++++++-------------- >> include/hw/xen/xen.h | 3 ++- >> xen-all.c | 24 ++++++++++++++---------- >> xen-stub.c | 3 ++- >> 5 files changed, 49 insertions(+), 41 deletions(-) >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >> index 7930a26..bd52f6e 100644 >> --- a/hw/i386/pc_piix.c >> +++ b/hw/i386/pc_piix.c >> @@ -96,21 +96,6 @@ static void pc_init1(QEMUMachineInitArgs *args, >> FWCfgState *fw_cfg = NULL; >> PcGuestInfo *guest_info; >> >> - if (xen_enabled() && xen_hvm_init(&ram_memory) != 0) { >> - fprintf(stderr, "xen hardware virtual machine initialisation failed\n"); >> - exit(1); >> - } >> - >> - icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); >> - object_property_add_child(qdev_get_machine(), "icc-bridge", >> - OBJECT(icc_bridge), NULL); >> - >> - pc_cpus_init(args->cpu_model, icc_bridge); >> - >> - if (kvm_enabled() && kvmclock_enabled) { >> - kvmclock_create(); >> - } >> - >> /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory). >> * If it doesn't, we need to split it in chunks below and above 4G. >> * In any case, try to make sure that guest addresses aligned at >> @@ -127,6 +112,22 @@ static void pc_init1(QEMUMachineInitArgs *args, >> below_4g_mem_size = args->ram_size; >> } >> >> + if (xen_enabled() && xen_hvm_init(&below_4g_mem_size, &above_4g_mem_size, >> + &ram_memory) != 0) { >> + fprintf(stderr, "xen hardware virtual machine initialisation failed\n"); >> + exit(1); >> + } >> + >> + icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); >> + object_property_add_child(qdev_get_machine(), "icc-bridge", >> + OBJECT(icc_bridge), NULL); >> + >> + pc_cpus_init(args->cpu_model, icc_bridge); >> + >> + if (kvm_enabled() && kvmclock_enabled) { >> + kvmclock_create(); >> + } >> + >> if (pci_enabled) { >> pci_memory = g_new(MemoryRegion, 1); >> memory_region_init(pci_memory, NULL, "pci", UINT64_MAX); > Movement looks okay to me, CC'ing Igor. Did you test KVM or only Xen? I have tested on one version of KVM (Fedora 17). > It would be nice to follow up replacing fprintf() with error_report() > though in a separate patch. Will do. -Don Slutz >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c >> index c844dc2..6e34fe1 100644 >> --- a/hw/i386/pc_q35.c >> +++ b/hw/i386/pc_q35.c >> @@ -83,20 +83,6 @@ static void pc_q35_init(QEMUMachineInitArgs *args) >> DeviceState *icc_bridge; >> PcGuestInfo *guest_info; >> >> - if (xen_enabled() && xen_hvm_init(&ram_memory) != 0) { >> - fprintf(stderr, "xen hardware virtual machine initialisation failed\n"); >> - exit(1); >> - } >> - >> - icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); >> - object_property_add_child(qdev_get_machine(), "icc-bridge", >> - OBJECT(icc_bridge), NULL); >> - >> - pc_cpus_init(args->cpu_model, icc_bridge); >> - pc_acpi_init("q35-acpi-dsdt.aml"); >> - >> - kvmclock_create(); >> - >> /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory >> * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping >> * also known as MMCFG). >> @@ -115,6 +101,21 @@ static void pc_q35_init(QEMUMachineInitArgs *args) >> below_4g_mem_size = args->ram_size; >> } >> >> + if (xen_enabled() && xen_hvm_init(&below_4g_mem_size, &above_4g_mem_size, >> + &ram_memory) != 0) { >> + fprintf(stderr, "xen hardware virtual machine initialisation failed\n"); >> + exit(1); >> + } >> + >> + icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); >> + object_property_add_child(qdev_get_machine(), "icc-bridge", >> + OBJECT(icc_bridge), NULL); >> + >> + pc_cpus_init(args->cpu_model, icc_bridge); >> + pc_acpi_init("q35-acpi-dsdt.aml"); >> + >> + kvmclock_create(); >> + >> /* pci enabled */ >> if (pci_enabled) { >> pci_memory = g_new(MemoryRegion, 1); > [snip] > > Dito here. > > Xen parts look sane to me. > > Reviewed-by: Andreas Färber > > Regards, > Andreas >