Re: [Qemu-devel] [PATCH] qtest: add a fuzz test to fdc-test

2012-05-21 Thread Paolo Bonzini
Il 21/05/2012 09:51, Kevin Wolf ha scritto:
  GTESTER check-qtest-i386
  FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
  FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
  FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x1f
  FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0xa8
  FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
  FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
  FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
  FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x37
  FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
  FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
  FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
  FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
  FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
  FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
  FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x93
  FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0xe4
  FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0xc1
  FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x92
  
  Maybe they should be fixed first.
 What do you mean by fixing? Turning them into DPRINTFs?

Or trace events?

Paolo



Re: [Qemu-devel] [PATCH] qtest: add a fuzz test to fdc-test

2012-05-21 Thread Kevin Wolf
Am 21.05.2012 10:11, schrieb Paolo Bonzini:
 Il 21/05/2012 09:51, Kevin Wolf ha scritto:
 GTESTER check-qtest-i386
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x1f
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0xa8
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x37
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x93
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0xe4
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0xc1
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x92

 Maybe they should be fixed first.
 What do you mean by fixing? Turning them into DPRINTFs?
 
 Or trace events?

Yeah, you could turn all FLOPPY_DPRINTFs into trace events. But the
point here is that today it's a FLOPPY_ERROR, and except for register
fuzzing they report real problems with the emulation and not just some
debugging information. So I'm not sure if hiding them is really a fix.

Kevin



Re: [Qemu-devel] [PATCH] qtest: add a fuzz test to fdc-test

2012-05-21 Thread Paolo Bonzini
  What do you mean by fixing? Turning them into DPRINTFs?
  
  Or trace events?
 
 Yeah, you could turn all FLOPPY_DPRINTFs into trace events. But the
 point here is that today it's a FLOPPY_ERROR, and except for register
 fuzzing they report real problems with the emulation and not just
 some debugging information. So I'm not sure if hiding them is really a
 fix.

It depends, controller not ready for reading is most likely just caused by
fuzzing.  Most unimplemented commands are also invalid on real hardware too.

Paolo



Re: [Qemu-devel] [PATCH] qtest: add a fuzz test to fdc-test

2012-05-21 Thread Kevin Wolf
Am 19.05.2012 14:54, schrieb Blue Swirl:
 Add a simple register fuzzing test to floppy controller tests.
 
 Signed-off-by: Blue Swirl blauwir...@gmail.com
 ---
 There's a lot of output like:
 GTESTER check-qtest-i386
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x1f
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0xa8
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x37
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x93
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0xe4
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0xc1
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x92
 
 Maybe they should be fixed first.

What do you mean by fixing? Turning them into DPRINTFs?

Kevin



Re: [Qemu-devel] [PATCH] qtest: add a fuzz test to fdc-test

2012-05-21 Thread Blue Swirl
On Mon, May 21, 2012 at 8:14 AM, Kevin Wolf kw...@redhat.com wrote:
 Am 21.05.2012 10:11, schrieb Paolo Bonzini:
 Il 21/05/2012 09:51, Kevin Wolf ha scritto:
 GTESTER check-qtest-i386
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x1f
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0xa8
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x37
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_read_data: controller not ready for reading
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x93
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0xe4
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0xc1
 FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0x92

 Maybe they should be fixed first.
 What do you mean by fixing? Turning them into DPRINTFs?

 Or trace events?

 Yeah, you could turn all FLOPPY_DPRINTFs into trace events. But the
 point here is that today it's a FLOPPY_ERROR, and except for register
 fuzzing they report real problems with the emulation and not just some
 debugging information. So I'm not sure if hiding them is really a fix.

While not a DoS, letting the guest spam the console at will is not
nice either. Maybe we need a new method to enable a selected set of
printouts, something like '-d unimplemented'. That way no recompiling
would be needed.


 Kevin



Re: [Qemu-devel] [PATCH] qtest: add a fuzz test to fdc-test

2012-05-21 Thread Blue Swirl
On Mon, May 21, 2012 at 8:18 AM, Paolo Bonzini pbonz...@redhat.com wrote:
  What do you mean by fixing? Turning them into DPRINTFs?
 
  Or trace events?

 Yeah, you could turn all FLOPPY_DPRINTFs into trace events. But the
 point here is that today it's a FLOPPY_ERROR, and except for register
 fuzzing they report real problems with the emulation and not just
 some debugging information. So I'm not sure if hiding them is really a
 fix.

 It depends, controller not ready for reading is most likely just caused by
 fuzzing.  Most unimplemented commands are also invalid on real hardware too.

Yes, but a malevolent guest could issue the same commands if those
could cause problems to QEMU.


 Paolo



Re: [Qemu-devel] [PATCH] qtest: add a fuzz test to fdc-test

2012-05-21 Thread Peter Maydell
On 21 May 2012 18:30, Blue Swirl blauwir...@gmail.com wrote:
 On Mon, May 21, 2012 at 8:14 AM, Kevin Wolf kw...@redhat.com wrote:
 Yeah, you could turn all FLOPPY_DPRINTFs into trace events. But the
 point here is that today it's a FLOPPY_ERROR, and except for register
 fuzzing they report real problems with the emulation and not just some
 debugging information. So I'm not sure if hiding them is really a fix.

 While not a DoS, letting the guest spam the console at will is not
 nice either. Maybe we need a new method to enable a selected set of
 printouts, something like '-d unimplemented'. That way no recompiling
 would be needed.

+1 for a better set of graduated logging/debug levels and a sensible
command line interface for turning them on and off. Possible severity
levels:
 * debug output
 * guest has done something that suggests it might be buggy, eg
   accessing nonexistent register
 * guest has tried to use something qemu doesn't implement
 * qemu (fatal) error

These should be orthogonal to the what area should we print logging
for question, I think.

With a clean API for defining log messages I think we could clean
up a lot of the legacy functions for asserting/aborting in various
ways (in particular a lot of the hw_error() uses).

-- PMM