RE: [Qemu-devel] patch for ne2000.c

2006-05-14 Thread Han, Zhu
Hi, Fabrice!
For your first comment, I have to say it's not a bug in the current OS. It's 
caused by the difference between ne2000's emulation and the real hardware 
detail. When the receive buffer is full and the receiving side has acknowledged 
the ENISR_RX signal, the hardware should raise the ENISR_OVER signal. But for 
the sake of simplicity, ne2000 don't implement ENISR_OVER semantic. And we 
really don't need any ENISR_OVER signal because we needn't do any recovery job. 
So, this is a workaround and the simplest way for this problem!

Best Regards, 
hanzhu
-Original Message-
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Fabrice Bellard
Sent: 2006年5月12日 5:52
To: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] patch for ne2000.c

OK for (2).

For (1) It would be good to find the exact behaviour of the NE2000 card.
Maybe ENISR_RX remain set as long are there are packets in the buffer ?
Otherwise your fix is a workaround to correct a bug in the OS driver...

Fabrice.

Han, Zhu wrote:
 Any comments for this patch?
 
 Best Regards, 
 hanzhu
 -Original Message-
 From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Han, Zhu
 Sent: 2006年5月9日 12:27
 To: qemu-devel@nongnu.org
 Subject: [Qemu-devel] patch for ne2000.c
 
 Hi, All!
 
 I'm a developer working on xen project! It's well known that xen has
 adopted a lot of codes and features from QEMU, especially the Device
 Mode Part!
 
 I fix a bug for ne2000 device emulation code in XEN and I expect it to
 be a potential bug for QEMU, either! Because you are all device mode
 experts, I submit this patch to you at first in order to ask you to
 review my patch. 
 
 Several notes:
 1) Because XEN use event driven mechanism in the main_loop(), irq may be
 missed due to the rather high speed and large file! For example, the
 ne2000_receive will filled up with the buffer and set up the ENISR_RX
 signal, however, the driver could ack and clear the ENISR_RX signal due
 to it could only handle a certain amount of packets once in it's
 interrupt handling routine!  The consequence for this specific steps is
 the netcard buffer is full but it never resend the ENISR_RX signal, at
 the last, the netcard will be halted! This problem could be rather rare
 for QEMU. Anyway, it's a potential bug.
 2) Many of the ne2000 spec said we should set boundary register should
 be set to indicate the last receive buffer page the host has read, and
 the driver in linux follows this guideline. So, we boundary == index,
 the buffer for the netcard is full and we can't write any packets into
 this buffer. This minor fix could prevent the ne2000 emulated card from
 overflow and destroying the previous received packet page! This problem
 could also be rare for QEMU since it could happen only under extreme
 circumstance! 
 
 Any feedbacks and comments will be appreciated! 
 
 --- qemu-snapshot-2006-05-07_23\hw\ne2000.c   Mon May 08 16:13:49 2006
 +++ ./ne2000.cMon May 08 16:57:33 2006
 @@ -159,9 +159,19 @@
  }
  }
  
 +static int ne2000_buffer_full(NE2000State *s);
  static void ne2000_update_irq(NE2000State *s)
  {
  int isr;
 +
 +if(ne2000_buffer_full(s)
 + !(s-isr  ENISR_RX)){
 + /* The freeing space is not enough, tell the ne2k driver
 +  * to fetch these packets!
 +  */
 +s-isr |= ENISR_RX;
 +}
 +
  isr = (s-isr  s-imr)  0x7f;
  #if defined(DEBUG_NE2000)
  printf(NE2000: Set IRQ line %d to %d (%02x %02x)\n,
 @@ -206,7 +216,10 @@
  
  index = s-curpag  8;
  boundary = s-boundary  8;
 -if (index  boundary)
 +if (index = boundary)
 + /* when index == boundary, we should assume 
 +  * the buffer is full instead of empty!
 +  */
  avail = boundary - index;
  else
  avail = (s-stop - s-start) - (index - boundary);
 
 Best Regards, 
 hanzhu
 
 
 ___
 Qemu-devel mailing list
 Qemu-devel@nongnu.org
 http://lists.nongnu.org/mailman/listinfo/qemu-devel
 
 



___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel


___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel


Re: [Qemu-devel] patch for ne2000.c

2006-05-11 Thread Fabrice Bellard
OK for (2).

For (1) It would be good to find the exact behaviour of the NE2000 card.
Maybe ENISR_RX remain set as long are there are packets in the buffer ?
Otherwise your fix is a workaround to correct a bug in the OS driver...

Fabrice.

Han, Zhu wrote:
 Any comments for this patch?
 
 Best Regards, 
 hanzhu
 -Original Message-
 From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Han, Zhu
 Sent: 2006年5月9日 12:27
 To: qemu-devel@nongnu.org
 Subject: [Qemu-devel] patch for ne2000.c
 
 Hi, All!
 
 I'm a developer working on xen project! It's well known that xen has
 adopted a lot of codes and features from QEMU, especially the Device
 Mode Part!
 
 I fix a bug for ne2000 device emulation code in XEN and I expect it to
 be a potential bug for QEMU, either! Because you are all device mode
 experts, I submit this patch to you at first in order to ask you to
 review my patch. 
 
 Several notes:
 1) Because XEN use event driven mechanism in the main_loop(), irq may be
 missed due to the rather high speed and large file! For example, the
 ne2000_receive will filled up with the buffer and set up the ENISR_RX
 signal, however, the driver could ack and clear the ENISR_RX signal due
 to it could only handle a certain amount of packets once in it's
 interrupt handling routine!  The consequence for this specific steps is
 the netcard buffer is full but it never resend the ENISR_RX signal, at
 the last, the netcard will be halted! This problem could be rather rare
 for QEMU. Anyway, it's a potential bug.
 2) Many of the ne2000 spec said we should set boundary register should
 be set to indicate the last receive buffer page the host has read, and
 the driver in linux follows this guideline. So, we boundary == index,
 the buffer for the netcard is full and we can't write any packets into
 this buffer. This minor fix could prevent the ne2000 emulated card from
 overflow and destroying the previous received packet page! This problem
 could also be rare for QEMU since it could happen only under extreme
 circumstance! 
 
 Any feedbacks and comments will be appreciated! 
 
 --- qemu-snapshot-2006-05-07_23\hw\ne2000.c   Mon May 08 16:13:49 2006
 +++ ./ne2000.cMon May 08 16:57:33 2006
 @@ -159,9 +159,19 @@
  }
  }
  
 +static int ne2000_buffer_full(NE2000State *s);
  static void ne2000_update_irq(NE2000State *s)
  {
  int isr;
 +
 +if(ne2000_buffer_full(s)
 + !(s-isr  ENISR_RX)){
 + /* The freeing space is not enough, tell the ne2k driver
 +  * to fetch these packets!
 +  */
 +s-isr |= ENISR_RX;
 +}
 +
  isr = (s-isr  s-imr)  0x7f;
  #if defined(DEBUG_NE2000)
  printf(NE2000: Set IRQ line %d to %d (%02x %02x)\n,
 @@ -206,7 +216,10 @@
  
  index = s-curpag  8;
  boundary = s-boundary  8;
 -if (index  boundary)
 +if (index = boundary)
 + /* when index == boundary, we should assume 
 +  * the buffer is full instead of empty!
 +  */
  avail = boundary - index;
  else
  avail = (s-stop - s-start) - (index - boundary);
 
 Best Regards, 
 hanzhu
 
 
 ___
 Qemu-devel mailing list
 Qemu-devel@nongnu.org
 http://lists.nongnu.org/mailman/listinfo/qemu-devel
 
 



___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel


RE: [Qemu-devel] patch for ne2000.c

2006-05-10 Thread Han, Zhu
Any comments for this patch?

Best Regards, 
hanzhu
-Original Message-
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Han, Zhu
Sent: 2006年5月9日 12:27
To: qemu-devel@nongnu.org
Subject: [Qemu-devel] patch for ne2000.c

Hi, All!

I'm a developer working on xen project! It's well known that xen has
adopted a lot of codes and features from QEMU, especially the Device
Mode Part!

I fix a bug for ne2000 device emulation code in XEN and I expect it to
be a potential bug for QEMU, either! Because you are all device mode
experts, I submit this patch to you at first in order to ask you to
review my patch. 

Several notes:
1) Because XEN use event driven mechanism in the main_loop(), irq may be
missed due to the rather high speed and large file! For example, the
ne2000_receive will filled up with the buffer and set up the ENISR_RX
signal, however, the driver could ack and clear the ENISR_RX signal due
to it could only handle a certain amount of packets once in it's
interrupt handling routine!  The consequence for this specific steps is
the netcard buffer is full but it never resend the ENISR_RX signal, at
the last, the netcard will be halted! This problem could be rather rare
for QEMU. Anyway, it's a potential bug.
2) Many of the ne2000 spec said we should set boundary register should
be set to indicate the last receive buffer page the host has read, and
the driver in linux follows this guideline. So, we boundary == index,
the buffer for the netcard is full and we can't write any packets into
this buffer. This minor fix could prevent the ne2000 emulated card from
overflow and destroying the previous received packet page! This problem
could also be rare for QEMU since it could happen only under extreme
circumstance! 

Any feedbacks and comments will be appreciated! 

--- qemu-snapshot-2006-05-07_23\hw\ne2000.c Mon May 08 16:13:49 2006
+++ ./ne2000.c  Mon May 08 16:57:33 2006
@@ -159,9 +159,19 @@
 }
 }
 
+static int ne2000_buffer_full(NE2000State *s);
 static void ne2000_update_irq(NE2000State *s)
 {
 int isr;
+
+if(ne2000_buffer_full(s)
+ !(s-isr  ENISR_RX)){
+   /* The freeing space is not enough, tell the ne2k driver
+* to fetch these packets!
+*/
+s-isr |= ENISR_RX;
+}
+
 isr = (s-isr  s-imr)  0x7f;
 #if defined(DEBUG_NE2000)
 printf(NE2000: Set IRQ line %d to %d (%02x %02x)\n,
@@ -206,7 +216,10 @@
 
 index = s-curpag  8;
 boundary = s-boundary  8;
-if (index  boundary)
+if (index = boundary)
+   /* when index == boundary, we should assume 
+* the buffer is full instead of empty!
+*/
 avail = boundary - index;
 else
 avail = (s-stop - s-start) - (index - boundary);

Best Regards, 
hanzhu


___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel


[Qemu-devel] patch for ne2000.c

2006-05-08 Thread Han, Zhu
Hi, All!

I'm a developer working on xen project! It's well known that xen has
adopted a lot of codes and features from QEMU, especially the Device
Mode Part!

I fix a bug for ne2000 device emulation code in XEN and I expect it to
be a potential bug for QEMU, either! Because you are all device mode
experts, I submit this patch to you at first in order to ask you to
review my patch. 

Several notes:
1) Because XEN use event driven mechanism in the main_loop(), irq may be
missed due to the rather high speed and large file! For example, the
ne2000_receive will filled up with the buffer and set up the ENISR_RX
signal, however, the driver could ack and clear the ENISR_RX signal due
to it could only handle a certain amount of packets once in it's
interrupt handling routine!  The consequence for this specific steps is
the netcard buffer is full but it never resend the ENISR_RX signal, at
the last, the netcard will be halted! This problem could be rather rare
for QEMU. Anyway, it's a potential bug.
2) Many of the ne2000 spec said we should set boundary register should
be set to indicate the last receive buffer page the host has read, and
the driver in linux follows this guideline. So, we boundary == index,
the buffer for the netcard is full and we can't write any packets into
this buffer. This minor fix could prevent the ne2000 emulated card from
overflow and destroying the previous received packet page! This problem
could also be rare for QEMU since it could happen only under extreme
circumstance! 

Any feedbacks and comments will be appreciated! 

--- qemu-snapshot-2006-05-07_23\hw\ne2000.c Mon May 08 16:13:49 2006
+++ ./ne2000.c  Mon May 08 16:57:33 2006
@@ -159,9 +159,19 @@
 }
 }
 
+static int ne2000_buffer_full(NE2000State *s);
 static void ne2000_update_irq(NE2000State *s)
 {
 int isr;
+
+if(ne2000_buffer_full(s)
+ !(s-isr  ENISR_RX)){
+   /* The freeing space is not enough, tell the ne2k driver
+* to fetch these packets!
+*/
+s-isr |= ENISR_RX;
+}
+
 isr = (s-isr  s-imr)  0x7f;
 #if defined(DEBUG_NE2000)
 printf(NE2000: Set IRQ line %d to %d (%02x %02x)\n,
@@ -206,7 +216,10 @@
 
 index = s-curpag  8;
 boundary = s-boundary  8;
-if (index  boundary)
+if (index = boundary)
+   /* when index == boundary, we should assume 
+* the buffer is full instead of empty!
+*/
 avail = boundary - index;
 else
 avail = (s-stop - s-start) - (index - boundary);

Best Regards, 
hanzhu



qemu_ne2000.patch
Description: qemu_ne2000.patch
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel