From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [PATCH 4/4] KVM: in-kernel-apic modification to QEMU Date: Fri, 11 May 2007 15:52:59 -0500 Message-ID: <4644D7AB.5040309@us.ibm.com> References: <20070511203316.22062.4417.stgit@novell1.haskins.net> <20070511204216.22062.97272.stgit@novell1.haskins.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Gregory Haskins Return-path: In-Reply-To: <20070511204216.22062.97272.stgit-sLgBBP33vUGnsjUZhwzVf9HuzzzSOjJt@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org Hi Gregory, The other patches all look good. Just some minor stuff here. Gregory Haskins wrote: > Signed-off-by: Gregory Haskins > --- > > @@ -888,10 +890,17 @@ static void ioapic_service(IOAPICState *s) > vector = pic_read_irq(isa_pic); > else > vector = entry & 0xff; > - > - apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode); > - apic_bus_deliver(deliver_bitmask, delivery_mode, > - vector, polarity, trig_mode); > + > + if (kvm_allowed && kvm_apic_level) { > + ext_apic_bus_deliver(dest, trig_mode, dest_mode, > + delivery_mode, vector); > + cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD); > + } else { > + apic_get_delivery_bitmask(deliver_bitmask, dest, > + dest_mode); > + apic_bus_deliver(deliver_bitmask, delivery_mode, > + vector, polarity, trig_mode); > + } > } > } > } > Could be my client, but I think you're spacing is off here. QEMU tends to use spaces instead of tabs and I think you're using tabs. > intno = pic_read_irq(isa_pic); > return intno; > @@ -483,9 +486,10 @@ static void pc_init1(int ram_size, int vga_ram_size, int boot_device, > } > register_savevm("cpu", i, 4, cpu_save, cpu_load, env); > qemu_register_reset(main_cpu_reset, env); > - if (pci_enabled) { > - apic_init(env); > - } > + if (!kvm_allowed || !kvm_apic_level) > + if (pci_enabled) { > + apic_init(env); > + } > } Can probably just fold all three conditions into a single if clause: if (!(kvm_allowed && kvm_apic_level) && pci_enabled) apic_init(env); Further, it may be worthwhile to just introduce a static inline to reduce it to: if (!use_kernel_apic() && pci_enabled) apic_init(env); > > /* allocate RAM */ > @@ -671,7 +675,7 @@ static void pc_init1(int ram_size, int vga_ram_size, int boot_device, > register_ioport_write(0x92, 1, 1, ioport92_write, NULL); > > if (pci_enabled) { > - ioapic = ioapic_init(); > + ioapic = ioapic_init(env); > } > Have you tested -smp 2 with -no-kvm by any chance? > diff --git a/qemu/vl.c b/qemu/vl.c > index 88e650e..986cea4 100644 > --- a/qemu/vl.c > +++ b/qemu/vl.c > @@ -7312,7 +7312,7 @@ int main(int argc, char **argv) > kvm_allowed = 0; > break; > case QEMU_OPTION_kvm_apic: > - kvm_apic_level = optarg; > + kvm_apic_level = atoi(optarg); > break; > #endif > Hrm, what are you diffing against? I don't see QEMU_OPTION_kvm_apic in my tree. Regards, Anthony Liguori > ------------------------------------------------------------------------- > This SF.net email is sponsored by DB2 Express > Download DB2 Express C - the FREE version of DB2 express and take > control of your XML. No limits. Just data. Click to get it now. > http://sourceforge.net/powerbar/db2/ > _______________________________________________ > kvm-devel mailing list > kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org > https://lists.sourceforge.net/lists/listinfo/kvm-devel > > ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/