Re: [Qemu-devel] [PATCH 2/2] fdc-test: Check RELATIVE SEEK beyond track 0/80

2012-07-24 Thread Blue Swirl
On Tue, Jul 24, 2012 at 9:35 AM, Kevin Wolf kw...@redhat.com wrote:
 Am 23.07.2012 19:09, schrieb Blue Swirl:
 On Tue, Jul 17, 2012 at 9:03 AM, Pavel Hrdina phrd...@redhat.com wrote:
 I tested it on the real floppy and the behavior is more complicated.

 This reminds me of an idea: it could be interesting to make qtest test
 real hardware, for example using a serial port to feed commands and
 get responses. The same protocol which libqtest uses could be
 interpreted by a boot loader level program or kernel module.

 The existing qtest cases wouldn't necessarily work there because they
 don't care about timing. Not sure if it's worth the effort writing real
 drivers in qtest cases just to be able to run them against real hardware
 occasionally.

It would be nice if the tests paid some attention to timing, though
QEMU does not implement many timing issues either.

The benefit from this setup would be that the test suite would be
validated against real HW. But as the hardware gets older and breaks
down, it will be increasingly difficult to validate the suite in the
future so I also think it's probably not worth the effort.


 Kevin



Re: [Qemu-devel] [PATCH 2/2] fdc-test: Check RELATIVE SEEK beyond track 0/80

2012-07-24 Thread Kevin Wolf
Am 23.07.2012 19:09, schrieb Blue Swirl:
 On Tue, Jul 17, 2012 at 9:03 AM, Pavel Hrdina phrd...@redhat.com wrote:
 I tested it on the real floppy and the behavior is more complicated.
 
 This reminds me of an idea: it could be interesting to make qtest test
 real hardware, for example using a serial port to feed commands and
 get responses. The same protocol which libqtest uses could be
 interpreted by a boot loader level program or kernel module.

The existing qtest cases wouldn't necessarily work there because they
don't care about timing. Not sure if it's worth the effort writing real
drivers in qtest cases just to be able to run them against real hardware
occasionally.

Kevin



Re: [Qemu-devel] [PATCH 2/2] fdc-test: Check RELATIVE SEEK beyond track 0/80

2012-07-23 Thread Blue Swirl
On Tue, Jul 17, 2012 at 9:03 AM, Pavel Hrdina phrd...@redhat.com wrote:
 On 07/16/2012 04:36 PM, Kevin Wolf wrote:

 TODO This needs to be checked against a real drive

 Signed-off-by: Kevin Wolfkw...@redhat.com
 ---
   tests/fdc-test.c |   48 +---
   1 files changed, 37 insertions(+), 11 deletions(-)

 diff --git a/tests/fdc-test.c b/tests/fdc-test.c
 index fa74411..56e745a 100644
 --- a/tests/fdc-test.c
 +++ b/tests/fdc-test.c
 @@ -94,13 +94,17 @@ static uint8_t floppy_recv(void)
   }
 /* pcn: Present Cylinder Number */
 -static void ack_irq(uint8_t *pcn)
 +static void ack_irq(uint8_t *st0, uint8_t *pcn)
   {
   uint8_t ret;
 g_assert(get_irq(FLOPPY_IRQ));
   floppy_send(CMD_SENSE_INT);
 -floppy_recv();
 +
 +ret = floppy_recv();
 +if (st0 != NULL) {
 +*st0 = ret;
 +}
 ret = floppy_recv();
   if (pcn != NULL) {
 @@ -175,7 +179,7 @@ static void send_seek(int cyl)
   floppy_send(head  2 | drive);
   g_assert(!get_irq(FLOPPY_IRQ));
   floppy_send(cyl);
 -ack_irq(NULL);
 +ack_irq(NULL, NULL);
   }
 static uint8_t cmos_read(uint8_t reg)
 @@ -295,29 +299,51 @@ static void test_relative_seek(void)
   {
   uint8_t drive = 0;
   uint8_t head = 0;
 -uint8_t cyl = 1;
   uint8_t pcn;
 +uint8_t st0;
 /* Send seek to track 0 */
   send_seek(0);
   -/* Send relative seek to increase track by 1 */
 +/* Send relative seek to increase track by 3 */
   floppy_send(CMD_RELATIVE_SEEK_IN);
   floppy_send(head  2 | drive);
   g_assert(!get_irq(FLOPPY_IRQ));
 -floppy_send(cyl);
 +floppy_send(3);
   -ack_irq(pcn);
 -g_assert(pcn == 1);
 +ack_irq(st0, pcn);
 +g_assert_cmpint(pcn, ==, 3);
 +g_assert_cmpint(st0, ==, 0x20);
 /* Send relative seek to decrease track by 1 */
   floppy_send(CMD_RELATIVE_SEEK_OUT);
   floppy_send(head  2 | drive);
   g_assert(!get_irq(FLOPPY_IRQ));
 -floppy_send(cyl);
 +floppy_send(1);
 +
 +ack_irq(st0, pcn);
 +g_assert_cmpint(pcn, ==, 2);
 +g_assert_cmpint(st0, ==, 0x20);
 +
 +/* Send relative seek to beyond track 0 */
 +floppy_send(CMD_RELATIVE_SEEK_OUT);
 +floppy_send(head  2 | drive);
 +g_assert(!get_irq(FLOPPY_IRQ));
 +floppy_send(42);
 +
 +ack_irq(st0, pcn);
 +g_assert_cmpint(pcn, ==, 0);
 +g_assert_cmpint(st0, ==, 0x70);
 +
 +/* Send try relative seek to beyond track 80 */
 +floppy_send(CMD_RELATIVE_SEEK_IN);
 +floppy_send(head  2 | drive);
 +g_assert(!get_irq(FLOPPY_IRQ));
 +floppy_send(200);
   -ack_irq(pcn);
 -g_assert(pcn == 0);
 +ack_irq(st0, pcn);
 +g_assert_cmpint(pcn, ==, 79);
 +g_assert_cmpint(st0, ==, 0x50);
   }
 /* success if no crash or abort */

 I tested it on the real floppy and the behavior is more complicated.

This reminds me of an idea: it could be interesting to make qtest test
real hardware, for example using a serial port to feed commands and
get responses. The same protocol which libqtest uses could be
interpreted by a boot loader level program or kernel module.


 Let's say that we start on the track 0 and the floppy media has 80 tracks.

 You send the relative_seek_in by 1 end the result is
 st0: 0x20
 rcn: 1.
 Then you issue the read_id command and you get
 st0: 0x00
 st1: 0x00
 pcn: 1.

 Then you send the relative_seek_in by 100 and the result is
 st0: 0x20
 rcn: 101
 but read_id returns
 st0: 0x40
 st1: 0x01
 pcn: 0.
 This probably means, that you are out of available tracks.

 Then you send the relative_seek_out by 100 and the result is
 st: 0x70
 rcn: 20
 but read_id returns
 st0: 0x00
 st1: 0x00
 pcn: 0.
 You return reading heads to the track 0, but the relative_seek_out returns
 that you are on the relative track 20.

 To get the floppy drive fully working, you have to send the seek to the
 track 0. If you don't do it, every seek to different track then the track 0
 will always seek to the track 0 and set rcn to track what you want. In this
 buggy state only the relative_seek will change the real track. The
 command read_id returns the real position of the reading heads. For
 example:

 Now you send seek to track 2 and you get
 st0: 0x20
 pcn: 2
 but if you send read_id the result is
 st0: 0x00
 st1: 0x00
 pcn: 0

 Pavel





Re: [Qemu-devel] [PATCH 2/2] fdc-test: Check RELATIVE SEEK beyond track 0/80

2012-07-17 Thread Pavel Hrdina

On 07/16/2012 04:36 PM, Kevin Wolf wrote:

TODO This needs to be checked against a real drive

Signed-off-by: Kevin Wolfkw...@redhat.com
---
  tests/fdc-test.c |   48 +---
  1 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index fa74411..56e745a 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -94,13 +94,17 @@ static uint8_t floppy_recv(void)
  }
  
  /* pcn: Present Cylinder Number */

-static void ack_irq(uint8_t *pcn)
+static void ack_irq(uint8_t *st0, uint8_t *pcn)
  {
  uint8_t ret;
  
  g_assert(get_irq(FLOPPY_IRQ));

  floppy_send(CMD_SENSE_INT);
-floppy_recv();
+
+ret = floppy_recv();
+if (st0 != NULL) {
+*st0 = ret;
+}
  
  ret = floppy_recv();

  if (pcn != NULL) {
@@ -175,7 +179,7 @@ static void send_seek(int cyl)
  floppy_send(head  2 | drive);
  g_assert(!get_irq(FLOPPY_IRQ));
  floppy_send(cyl);
-ack_irq(NULL);
+ack_irq(NULL, NULL);
  }
  
  static uint8_t cmos_read(uint8_t reg)

@@ -295,29 +299,51 @@ static void test_relative_seek(void)
  {
  uint8_t drive = 0;
  uint8_t head = 0;
-uint8_t cyl = 1;
  uint8_t pcn;
+uint8_t st0;
  
  /* Send seek to track 0 */

  send_seek(0);
  
-/* Send relative seek to increase track by 1 */

+/* Send relative seek to increase track by 3 */
  floppy_send(CMD_RELATIVE_SEEK_IN);
  floppy_send(head  2 | drive);
  g_assert(!get_irq(FLOPPY_IRQ));
-floppy_send(cyl);
+floppy_send(3);
  
-ack_irq(pcn);

-g_assert(pcn == 1);
+ack_irq(st0, pcn);
+g_assert_cmpint(pcn, ==, 3);
+g_assert_cmpint(st0, ==, 0x20);
  
  /* Send relative seek to decrease track by 1 */

  floppy_send(CMD_RELATIVE_SEEK_OUT);
  floppy_send(head  2 | drive);
  g_assert(!get_irq(FLOPPY_IRQ));
-floppy_send(cyl);
+floppy_send(1);
+
+ack_irq(st0, pcn);
+g_assert_cmpint(pcn, ==, 2);
+g_assert_cmpint(st0, ==, 0x20);
+
+/* Send relative seek to beyond track 0 */
+floppy_send(CMD_RELATIVE_SEEK_OUT);
+floppy_send(head  2 | drive);
+g_assert(!get_irq(FLOPPY_IRQ));
+floppy_send(42);
+
+ack_irq(st0, pcn);
+g_assert_cmpint(pcn, ==, 0);
+g_assert_cmpint(st0, ==, 0x70);
+
+/* Send try relative seek to beyond track 80 */
+floppy_send(CMD_RELATIVE_SEEK_IN);
+floppy_send(head  2 | drive);
+g_assert(!get_irq(FLOPPY_IRQ));
+floppy_send(200);
  
-ack_irq(pcn);

-g_assert(pcn == 0);
+ack_irq(st0, pcn);
+g_assert_cmpint(pcn, ==, 79);
+g_assert_cmpint(st0, ==, 0x50);
  }
  
  /* success if no crash or abort */

I tested it on the real floppy and the behavior is more complicated.

Let's say that we start on the track 0 and the floppy media has 80 tracks.

You send the relative_seek_in by 1 end the result is
st0: 0x20
rcn: 1.
Then you issue the read_id command and you get
st0: 0x00
st1: 0x00
pcn: 1.

Then you send the relative_seek_in by 100 and the result is
st0: 0x20
rcn: 101
but read_id returns
st0: 0x40
st1: 0x01
pcn: 0.
This probably means, that you are out of available tracks.

Then you send the relative_seek_out by 100 and the result is
st: 0x70
rcn: 20
but read_id returns
st0: 0x00
st1: 0x00
pcn: 0.
You return reading heads to the track 0, but the relative_seek_out 
returns that you are on the relative track 20.


To get the floppy drive fully working, you have to send the seek to 
the track 0. If you don't do it, every seek to different track then the 
track 0 will always seek to the track 0 and set rcn to track what you 
want. In this buggy state only the relative_seek will change the 
real track. The command read_id returns the real position of the 
reading heads. For example:


Now you send seek to track 2 and you get
st0: 0x20
pcn: 2
but if you send read_id the result is
st0: 0x00
st1: 0x00
pcn: 0

Pavel




[Qemu-devel] [PATCH 2/2] fdc-test: Check RELATIVE SEEK beyond track 0/80

2012-07-16 Thread Kevin Wolf
TODO This needs to be checked against a real drive

Signed-off-by: Kevin Wolf kw...@redhat.com
---
 tests/fdc-test.c |   48 +---
 1 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index fa74411..56e745a 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -94,13 +94,17 @@ static uint8_t floppy_recv(void)
 }
 
 /* pcn: Present Cylinder Number */
-static void ack_irq(uint8_t *pcn)
+static void ack_irq(uint8_t *st0, uint8_t *pcn)
 {
 uint8_t ret;
 
 g_assert(get_irq(FLOPPY_IRQ));
 floppy_send(CMD_SENSE_INT);
-floppy_recv();
+
+ret = floppy_recv();
+if (st0 != NULL) {
+*st0 = ret;
+}
 
 ret = floppy_recv();
 if (pcn != NULL) {
@@ -175,7 +179,7 @@ static void send_seek(int cyl)
 floppy_send(head  2 | drive);
 g_assert(!get_irq(FLOPPY_IRQ));
 floppy_send(cyl);
-ack_irq(NULL);
+ack_irq(NULL, NULL);
 }
 
 static uint8_t cmos_read(uint8_t reg)
@@ -295,29 +299,51 @@ static void test_relative_seek(void)
 {
 uint8_t drive = 0;
 uint8_t head = 0;
-uint8_t cyl = 1;
 uint8_t pcn;
+uint8_t st0;
 
 /* Send seek to track 0 */
 send_seek(0);
 
-/* Send relative seek to increase track by 1 */
+/* Send relative seek to increase track by 3 */
 floppy_send(CMD_RELATIVE_SEEK_IN);
 floppy_send(head  2 | drive);
 g_assert(!get_irq(FLOPPY_IRQ));
-floppy_send(cyl);
+floppy_send(3);
 
-ack_irq(pcn);
-g_assert(pcn == 1);
+ack_irq(st0, pcn);
+g_assert_cmpint(pcn, ==, 3);
+g_assert_cmpint(st0, ==, 0x20);
 
 /* Send relative seek to decrease track by 1 */
 floppy_send(CMD_RELATIVE_SEEK_OUT);
 floppy_send(head  2 | drive);
 g_assert(!get_irq(FLOPPY_IRQ));
-floppy_send(cyl);
+floppy_send(1);
+
+ack_irq(st0, pcn);
+g_assert_cmpint(pcn, ==, 2);
+g_assert_cmpint(st0, ==, 0x20);
+
+/* Send relative seek to beyond track 0 */
+floppy_send(CMD_RELATIVE_SEEK_OUT);
+floppy_send(head  2 | drive);
+g_assert(!get_irq(FLOPPY_IRQ));
+floppy_send(42);
+
+ack_irq(st0, pcn);
+g_assert_cmpint(pcn, ==, 0);
+g_assert_cmpint(st0, ==, 0x70);
+
+/* Send try relative seek to beyond track 80 */
+floppy_send(CMD_RELATIVE_SEEK_IN);
+floppy_send(head  2 | drive);
+g_assert(!get_irq(FLOPPY_IRQ));
+floppy_send(200);
 
-ack_irq(pcn);
-g_assert(pcn == 0);
+ack_irq(st0, pcn);
+g_assert_cmpint(pcn, ==, 79);
+g_assert_cmpint(st0, ==, 0x50);
 }
 
 /* success if no crash or abort */
-- 
1.7.6.5