Re: e1000 and PXE issues

2008-07-26 Thread Avi Kivity

Greg Kurtzer wrote:

Hello,

I noticed some problems with the e1000 implementation in kvm = 70. At
first glance it seemed liked a PXE problem as it would not acknowledge
the DHCP offer from the server. I tried several different Etherboot
ROM images and version 5.2.6 seemed to work. That version isn't PXE
compliant so I built an ELF image to boot, and it downloaded it very,
very, very, very slowly (as in about 10 minutes) but it did end up
working.

This all worked perfectly with version 69 and previous.
  


There are two patches to e1000 in kvm-70; can you try backing them out 
(patch -Rp1  test.patch) to see which one is guilty?


Candidates attached.


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

commit 2ec717a98c8b8e0c8f1ddc562115ca81d2dc024e
Author: Laurent Vivier [EMAIL PROTECTED]
Date:   Fri May 30 16:07:31 2008 +0200

kvm: qemu: coalesced MMIO support (e1000)

This patch defines coalesced MMIO zones for e1000 ethernet card.

Signed-off-by: Laurent Vivier [EMAIL PROTECTED]
Signed-off-by: Avi Kivity [EMAIL PROTECTED]

diff --git a/qemu/hw/e1000.c b/qemu/hw/e1000.c
index 01f8983..5b3a365 100644
--- a/qemu/hw/e1000.c
+++ b/qemu/hw/e1000.c
@@ -26,6 +26,7 @@
 #include hw.h
 #include pci.h
 #include net.h
+#include qemu-kvm.h
 
 #include e1000_hw.h
 
@@ -938,6 +939,18 @@ e1000_mmio_map(PCIDevice *pci_dev, int region_num,
 
 d-mmio_base = addr;
 cpu_register_physical_memory(addr, PNPMMIO_SIZE, d-mmio_index);
+
+if (kvm_enabled()) {
+	int i;
+uint32_t excluded_regs[] = {
+E1000_MDIC, E1000_ICR, E1000_ICS, E1000_IMS,
+E1000_IMC, E1000_TCTL, E1000_TDT, PNPMMIO_SIZE
+};
+qemu_kvm_register_coalesced_mmio(addr, excluded_regs[0]);
+for (i = 0; excluded_regs[i] != PNPMMIO_SIZE; i++)
+qemu_kvm_register_coalesced_mmio(addr + excluded_regs[i] + 4,
+ excluded_regs[i + 1] - excluded_regs[i] - 4);
+}
 }
 
 static int
commit 404ce95cb202ca6015beb26e9b870f91c0309ee0
Author: Anthony Liguori [EMAIL PROTECTED]
Date:   Wed May 7 16:40:58 2008 -0500

kvm: qemu: fix e1000 can_receive handler

The current logic of the can_receive handler is to allow packets whenever the
receiver is disabled or when there are descriptors available in the ring.

I think the logic ought to be to allow packets whenever the receiver is enabled
and there are descriptors available in the ring.

Signed-off-by: Anthony Liguori [EMAIL PROTECTED]
Signed-off-by: Avi Kivity [EMAIL PROTECTED]

diff --git a/qemu/hw/e1000.c b/qemu/hw/e1000.c
index 0728539..01f8983 100644
--- a/qemu/hw/e1000.c
+++ b/qemu/hw/e1000.c
@@ -520,8 +520,8 @@ e1000_can_receive(void *opaque)
 {
 E1000State *s = opaque;
 
-return (!(s-mac_reg[RCTL]  E1000_RCTL_EN) ||
-s-mac_reg[RDH] != s-mac_reg[RDT]);
+return ((s-mac_reg[RCTL]  E1000_RCTL_EN) 
+	s-mac_reg[RDH] != s-mac_reg[RDT]);
 }
 
 static void


Re: [PATCH 1/4] KVM: Add irq ack notifier list

2008-07-26 Thread Avi Kivity

Ben-Ami Yassour wrote:

From: Avi Kivity [EMAIL PROTECTED]

This can be used by kvm subsystems that are interested in when
interrupts
are acked, for example time drift compenstation.

  


Please add the pic and ioapic glue code to this patch.


Signed-off-by: Avi Kivity [EMAIL PROTECTED]
  


(You need to add your signoff to patches you send; even if you didn't 
modify them).



--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH retry] arch/ia64/kvm/kvm-ia64.c: Add local_irq_restore in error handling code

2008-07-26 Thread Avi Kivity

Julia Lawall wrote:

From: Julia Lawall [EMAIL PROTECTED]

There is a call to local_irq_restore in the normal exit case, so it would
seem that there should be one on an error return as well.

The semantic patch that finds this problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)
  


Applied, thanks.

--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/9] kvm: qemu: Fix virtio_net tx timer

2008-07-26 Thread Avi Kivity

Mark McLoughlin wrote:

The current virtio_net tx timer is 2ns, which doesn't
make any sense. Set it to a more reasonable 150us
instead.

Signed-off-by: Mark McLoughlin [EMAIL PROTECTED]
---
 qemu/hw/virtio-net.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
index 2e57e5a..31867f1 100644
--- a/qemu/hw/virtio-net.c
+++ b/qemu/hw/virtio-net.c
@@ -26,7 +26,7 @@
 #define VIRTIO_NET_F_MAC   5
 #define VIRTIO_NET_F_GS0   6
 
-#define TX_TIMER_INTERVAL (1000 / 500)

+#define TX_TIMER_INTERVAL (15) /* 150 us */
 
  


Ouch.


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: e1000 and PXE issues

2008-07-26 Thread Greg Kurtzer
On Sat, Jul 26, 2008 at 1:12 AM, Avi Kivity [EMAIL PROTECTED] wrote:
 Greg Kurtzer wrote:

 Hello,

 I noticed some problems with the e1000 implementation in kvm = 70. At
 first glance it seemed liked a PXE problem as it would not acknowledge
 the DHCP offer from the server. I tried several different Etherboot
 ROM images and version 5.2.6 seemed to work. That version isn't PXE
 compliant so I built an ELF image to boot, and it downloaded it very,
 very, very, very slowly (as in about 10 minutes) but it did end up
 working.

 This all worked perfectly with version 69 and previous.


 There are two patches to e1000 in kvm-70; can you try backing them out
 (patch -Rp1  test.patch) to see which one is guilty?

 Candidates attached.

Reverted both of these patches yet the problem remains. :(

This is easy to reproduce (at least on my build). if there is a DHCP
server on the network, just do a network boot on the e1000. It makes a
correct DHCPDISCOVER, but never responds to the DHCPOFFER (it should
do a DHCPREQUEST next). No packets are getting lost according tcpdump
on the master.

The console is showing:

Probing pci nic...
[e1000-82540em]Ethernet addr: 00:04:21:DE:99:55
Searching for server (DHCP)No IP address
.No IP address
.No IP address
.No IP address
.No IP address
.No IP address
.No IP address
...

Thanks again for the help!

-- 
Greg Kurtzer
http://www.infiscale.com/
http://www.runlevelzero.net/
http://www.perceus.org/
http://www.caoslinux.org/
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/9][RFC] KVM virtio_net performance

2008-07-26 Thread Bill Davidsen

Anthony Liguori wrote:

Hi Mark,

Mark McLoughlin wrote:

Hey,
  Here's a bunch of patches attempting to improve the performance
of virtio_net. This is more an RFC rather than a patch submission
since, as can be seen below, not all patches actually improve the
perfomance measurably.
  


I'm still seeing the same problem I saw with my patch series.  Namely, 
dhclient fails to get a DHCP address.  Rusty noticed that RX has a lot 
more packets received then it should so we're suspicious that we're 
getting packet corruption.


I have been discussing this (on this list) in another thread. Putting 
tcpdump on the eth0 device in the VM, the br0 device in the host, and 
the eth0 (physical NIC) in the host, you can see that when the VM 
generates a DHCP request it shows up on the br0 in the host, but never 
gets sent on the wire by eth0.


That's the point of failure, at least using RHEL5/FC6/kvm-66 as the 
environment.


Configuring the tap device with a static address, here's what I get with 
iperf:


w/o patches:

guest-host: 625 Mbits/sec
host-guest: 825 Mbits/sec

w/patches

guest-host:  2.02 Gbits/sec
host-guest: 1.89 Gbits/sec

guest lo: 4.35 Gbits/sec
host lo: 4.36 Gbits/sec

This is with KVM GUEST configured FWIW.

Regards,

Anthony Liguori



--
Bill Davidsen [EMAIL PROTECTED]
  We have more to fear from the bungling of the incompetent than from
the machinations of the wicked.  - from Slashdot
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[patch 0/3] fix PIT injection

2008-07-26 Thread Marcelo Tosatti
The in-kernel PIT emulation can either inject too many or too few
interrupts.

-- 

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[patch 1/3] KVM: Add irq ack notifier list

2008-07-26 Thread Marcelo Tosatti
From: Avi Kivity [EMAIL PROTECTED]

This can be used by kvm subsystems that are interested in when
interrupts are acked, for example time drift compensation.

Signed-off-by: Avi Kivity [EMAIL PROTECTED]
---
 arch/x86/kvm/irq.c |   22 ++
 arch/x86/kvm/irq.h |5 +
 include/asm-x86/kvm_host.h |7 +++
 3 files changed, 34 insertions(+), 0 deletions(-)


Index: kvm/arch/x86/kvm/irq.c
===
--- kvm.orig/arch/x86/kvm/irq.c
+++ kvm/arch/x86/kvm/irq.c
@@ -111,3 +111,25 @@ void kvm_set_irq(struct kvm *kvm, int ir
kvm_ioapic_set_irq(kvm-arch.vioapic, irq, level);
kvm_pic_set_irq(pic_irqchip(kvm), irq, level);
 }
+
+void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi)
+{
+   struct kvm_irq_ack_notifier *kian;
+   struct hlist_node *n;
+
+   hlist_for_each_entry(kian, n, kvm-arch.irq_ack_notifier_list, link)
+   if (kian-gsi == gsi)
+   kian-irq_acked(kian);
+}
+
+void kvm_register_irq_ack_notifier(struct kvm *kvm,
+  struct kvm_irq_ack_notifier *kian)
+{
+   hlist_add_head(kian-link, kvm-arch.irq_ack_notifier_list);
+}
+
+void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
+struct kvm_irq_ack_notifier *kian)
+{
+   hlist_del(kian-link);
+}
Index: kvm/arch/x86/kvm/irq.h
===
--- kvm.orig/arch/x86/kvm/irq.h
+++ kvm/arch/x86/kvm/irq.h
@@ -83,6 +83,11 @@ static inline int irqchip_in_kernel(stru
 void kvm_pic_reset(struct kvm_kpic_state *s);
 
 void kvm_set_irq(struct kvm *kvm, int irq, int level);
+void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi);
+void kvm_register_irq_ack_notifier(struct kvm *kvm,
+  struct kvm_irq_ack_notifier *kian);
+void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
+struct kvm_irq_ack_notifier *kian);
 
 void kvm_timer_intr_post(struct kvm_vcpu *vcpu, int vec);
 void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
Index: kvm/include/asm-x86/kvm_host.h
===
--- kvm.orig/include/asm-x86/kvm_host.h
+++ kvm/include/asm-x86/kvm_host.h
@@ -319,6 +319,12 @@ struct kvm_mem_alias {
gfn_t target_gfn;
 };
 
+struct kvm_irq_ack_notifier {
+   struct hlist_node link;
+   unsigned gsi;
+   void (*irq_acked)(struct kvm_irq_ack_notifier *kian);
+};
+
 struct kvm_arch{
int naliases;
struct kvm_mem_alias aliases[KVM_ALIAS_SLOTS];
@@ -334,6 +340,7 @@ struct kvm_arch{
struct kvm_pic *vpic;
struct kvm_ioapic *vioapic;
struct kvm_pit *vpit;
+   struct hlist_head irq_ack_notifier_list;
 
int round_robin_prev_vcpu;
unsigned int tss_addr;

-- 

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[patch 2/3] KVM: irq ack notification

2008-07-26 Thread Marcelo Tosatti
Based on a patch from: Ben-Ami Yassour [EMAIL PROTECTED]
which was based on a patch from: Amit Shah [EMAIL PROTECTED]

Notify IRQ acking on PIC/APIC emulation. The previous patch missed two things:

- Edge triggered interrupts on IOAPIC
- PIC reset with IRR/ISR set should be equivalent to ack (LAPIC probably
needs something similar).

Signed-off-by: Marcelo Tosatti [EMAIL PROTECTED]
CC: Amit Shah [EMAIL PROTECTED]
CC: Ben-Ami Yassour [EMAIL PROTECTED]

Index: kvm/arch/x86/kvm/i8259.c
===
--- kvm.orig/arch/x86/kvm/i8259.c
+++ kvm/arch/x86/kvm/i8259.c
@@ -159,9 +159,10 @@ static inline void pic_intack(struct kvm
s-irr = ~(1  irq);
 }
 
-int kvm_pic_read_irq(struct kvm_pic *s)
+int kvm_pic_read_irq(struct kvm *kvm)
 {
int irq, irq2, intno;
+   struct kvm_pic *s = pic_irqchip(kvm);
 
irq = pic_get_irq(s-pics[0]);
if (irq = 0) {
@@ -187,12 +188,21 @@ int kvm_pic_read_irq(struct kvm_pic *s)
intno = s-pics[0].irq_base + irq;
}
pic_update_irq(s);
+   kvm_notify_acked_irq(kvm, irq);
 
return intno;
 }
 
 void kvm_pic_reset(struct kvm_kpic_state *s)
 {
+   int irq;
+   struct kvm *kvm = s-pics_state-irq_request_opaque;
+
+   for (irq = 0; irq  PIC_NUM_PINS; irq++) {
+   if (!(s-imr  (1  irq))  (s-irr  (1  irq) ||
+   s-isr  (1  irq)))
+   kvm_notify_acked_irq(kvm, irq);
+   }
s-last_irr = 0;
s-irr = 0;
s-imr = 0;
Index: kvm/arch/x86/kvm/irq.c
===
--- kvm.orig/arch/x86/kvm/irq.c
+++ kvm/arch/x86/kvm/irq.c
@@ -72,7 +72,7 @@ int kvm_cpu_get_interrupt(struct kvm_vcp
if (kvm_apic_accept_pic_intr(v)) {
s = pic_irqchip(v-kvm);
s-output = 0;  /* PIC */
-   vector = kvm_pic_read_irq(s);
+   vector = kvm_pic_read_irq(v-kvm);
}
}
return vector;
Index: kvm/arch/x86/kvm/irq.h
===
--- kvm.orig/arch/x86/kvm/irq.h
+++ kvm/arch/x86/kvm/irq.h
@@ -63,11 +63,12 @@ struct kvm_pic {
void *irq_request_opaque;
int output; /* intr from master PIC */
struct kvm_io_device dev;
+   void (*ack_notifier)(void *opaque, int irq);
 };
 
 struct kvm_pic *kvm_create_pic(struct kvm *kvm);
 void kvm_pic_set_irq(void *opaque, int irq, int level);
-int kvm_pic_read_irq(struct kvm_pic *s);
+int kvm_pic_read_irq(struct kvm *kvm);
 void kvm_pic_update_irq(struct kvm_pic *s);
 
 static inline struct kvm_pic *pic_irqchip(struct kvm *kvm)
Index: kvm/virt/kvm/ioapic.c
===
--- kvm.orig/virt/kvm/ioapic.c
+++ kvm/virt/kvm/ioapic.c
@@ -39,6 +39,7 @@
 
 #include ioapic.h
 #include lapic.h
+#include irq.h
 
 #if 0
 #define ioapic_debug(fmt,arg...) printk(KERN_WARNING fmt,##arg)
@@ -285,26 +286,31 @@ void kvm_ioapic_set_irq(struct kvm_ioapi
}
 }
 
-static void __kvm_ioapic_update_eoi(struct kvm_ioapic *ioapic, int gsi)
+static void __kvm_ioapic_update_eoi(struct kvm_ioapic *ioapic, int gsi,
+   int trigger_mode)
 {
union ioapic_redir_entry *ent;
 
ent = ioapic-redirtbl[gsi];
-   ASSERT(ent-fields.trig_mode == IOAPIC_LEVEL_TRIG);
 
-   ent-fields.remote_irr = 0;
-   if (!ent-fields.mask  (ioapic-irr  (1  gsi)))
-   ioapic_service(ioapic, gsi);
+   kvm_notify_acked_irq(ioapic-kvm, gsi);
+
+   if (trigger_mode == IOAPIC_LEVEL_TRIG) {
+   ASSERT(ent-fields.trig_mode == IOAPIC_LEVEL_TRIG);
+   ent-fields.remote_irr = 0;
+   if (!ent-fields.mask  (ioapic-irr  (1  gsi)))
+   ioapic_service(ioapic, gsi);
+   }
 }
 
-void kvm_ioapic_update_eoi(struct kvm *kvm, int vector)
+void kvm_ioapic_update_eoi(struct kvm *kvm, int vector, int trigger_mode)
 {
struct kvm_ioapic *ioapic = kvm-arch.vioapic;
int i;
 
for (i = 0; i  IOAPIC_NUM_PINS; i++)
if (ioapic-redirtbl[i].fields.vector == vector)
-   __kvm_ioapic_update_eoi(ioapic, i);
+   __kvm_ioapic_update_eoi(ioapic, i, trigger_mode);
 }
 
 static int ioapic_in_range(struct kvm_io_device *this, gpa_t addr,
Index: kvm/virt/kvm/ioapic.h
===
--- kvm.orig/virt/kvm/ioapic.h
+++ kvm/virt/kvm/ioapic.h
@@ -58,6 +58,7 @@ struct kvm_ioapic {
} redirtbl[IOAPIC_NUM_PINS];
struct kvm_io_device dev;
struct kvm *kvm;
+   void (*ack_notifier)(void *opaque, int irq);
 };
 
 #ifdef DEBUG
@@ -87,7 +88,7 @@ static inline int irqchip_in_kernel(stru
 
 struct kvm_vcpu *kvm_get_lowest_prio_vcpu(struct kvm *kvm, u8 vector,