[Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Hannes Reinecke]

Hi all,

this patchset implements an emulation for the megaraid_sas HBA.
It provides emulates an LSI MegaRAID SAS 8708EM2 HBA, ie
presenting to the guest a virtual SCSI adapter.
Internally it is using aio for read/write requests and
either SG_IO or SCSI command emulation for everything else.

The reason for choosing the megaraid_sas HBA and not, say,
implementing a virtio scsi interface is because:
- the megaraid_sas is using a very simple firmware interface,
  comparable to virtio
- the HBA driver are already existent, so I only have to
  write the backend :-)

The device can be accessed by

-drive if=raid,file=XXX

In order to support SCSI command emulation I had to update /
patch up the existing SCSI disk support. This might be
not to everyones taste, so I'm open to alternative
suggestions.

But I certainly do _not_ want to update the SCSI disk
emulation, as this is really quite tied to the SCSI parallel
interface used by the old lsi53c895a.c.
Plus it doesn't do scatter-gather list handling, which
is quite impossible to fix without proper documentation.

Of course, if anyone else would step in here, I won't object :-)

It currently runs guests with 2.6.27 and up; Windows XP
support is not quite there yet. Anything else might work;
if not, enable debugging and sent me the logfile.

As usual, comment / suggestions  etc welcome.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Gerd Hoffmann]

   Hi,

> The device can be accessed by
>
> -drive if=raid,file=XXX

Don't extend that qemu automagic please.  The new way to handle this is:

   -drive if=none,id=mydisk,file=/path/to/some/disk.img
   -device megasas,id=raid
   -device scsi-disk,bus=raid.0,scsi-id=1,drive=mydisk

> In order to support SCSI command emulation I had to update /
> patch up the existing SCSI disk support. This might be
> not to everyones taste, so I'm open to alternative
> suggestions.
>
> But I certainly do _not_ want to update the SCSI disk
> emulation, as this is really quite tied to the SCSI parallel
> interface used by the old lsi53c895a.c.

--verbose please.  I'd prefer to fix scsi-disk bugs and/or limitations 
instead of working around them.

> Plus it doesn't do scatter-gather list handling,

Which should be fixed anyway.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Hannes Reinecke]

Am Tue 27 Oct 2009 05:47:21 PM CET schrieb Gerd Hoffmann <kraxel@redhat.com>:

>   Hi,
>
>> The device can be accessed by
>>
>> -drive if=raid,file=XXX
>
> Don't extend that qemu automagic please.  The new way to handle this is:
>
>   -drive if=none,id=mydisk,file=/path/to/some/disk.img
>   -device megasas,id=raid
>   -device scsi-disk,bus=raid.0,scsi-id=1,drive=mydisk
>
Alright, no problem there. Didn't know this.

>> In order to support SCSI command emulation I had to update /
>> patch up the existing SCSI disk support. This might be
>> not to everyones taste, so I'm open to alternative
>> suggestions.
>>
>> But I certainly do _not_ want to update the SCSI disk
>> emulation, as this is really quite tied to the SCSI parallel
>> interface used by the old lsi53c895a.c.
>
> --verbose please.  I'd prefer to fix scsi-disk bugs and/or limitations
> instead of working around them.
>
The problem is I don't have any documentation for the LSI parallel
SCSI controller. So I don't know if and in what shape I/O is passed
down, nor anything else. And as the SCSI disk emulation is really
tied into the LSI parallel SCSI controller, any change in the former
is likely to break the latter.
And what with me no way of fixing it. Hence I decided on this approach.

I surely can go ahead and patch up the scsi disk emulation, but it's
quite likely to break the LSI controller. If that's okay with everybody,
I'll surely go ahead there.

>> Plus it doesn't do scatter-gather list handling,
>
> Which should be fixed anyway.
>
Quite. But as I said, the LSI parallel SCSI controller is going to
suffer.

Cheers,

Hannes

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Avi Kivity]

On 10/28/2009 10:11 AM, Hannes Reinecke wrote:
>
> The problem is I don't have any documentation for the LSI parallel
> SCSI controller. So I don't know if and in what shape I/O is passed
> down, nor anything else. And as the SCSI disk emulation is really
> tied into the LSI parallel SCSI controller, any change in the former
> is likely to break the latter.
> And what with me no way of fixing it. Hence I decided on this approach.
>
> I surely can go ahead and patch up the scsi disk emulation, but it's
> quite likely to break the LSI controller. If that's okay with everybody,
> I'll surely go ahead there.

Reverse engineered devices are dangerous.  They might work on some 
guests but break badly on others, or even worse, break when a guest 
driver is updated.

Is there no way to get the documentation?

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

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Christoph Hellwig]

On Wed, Oct 28, 2009 at 09:11:29AM +0100, Hannes Reinecke wrote:
> The problem is I don't have any documentation for the LSI parallel
> SCSI controller. So I don't know if and in what shape I/O is passed
> down, nor anything else. And as the SCSI disk emulation is really
> tied into the LSI parallel SCSI controller, any change in the former
> is likely to break the latter.

scsi-disk is not tied into the lsi/symbios driver.  It's also used
by the esp driver and usb-mcd.  But it's a complete mess, and all my
attempts to untangle it have so far failed with my brain hurting for
a long time after.

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Gerd Hoffmann]

   Hi,

>>> In order to support SCSI command emulation I had to update /
>>> patch up the existing SCSI disk support. This might be
>>> not to everyones taste, so I'm open to alternative
>>> suggestions.
>>>
>>> But I certainly do _not_ want to update the SCSI disk
>>> emulation, as this is really quite tied to the SCSI parallel
>>> interface used by the old lsi53c895a.c.
>>
>> --verbose please. I'd prefer to fix scsi-disk bugs and/or limitations
>> instead of working around them.
>>
> The problem is I don't have any documentation for the LSI parallel
> SCSI controller. So I don't know if and in what shape I/O is passed
> down, nor anything else.

[ after briefly checking the code ]

Hmm.  Data is passed back+forth between scsi-device and scsi-adapter 
using a bounce buffer per request and a amazing maze of callbacks ...

That interface needs some serious rework, so we have a chance to kill 
the memcpy() and use iovecs.

> And as the SCSI disk emulation is really
> tied into the LSI parallel SCSI controller, any change in the former
> is likely to break the latter.

Not really.

> And what with me no way of fixing it. Hence I decided on this approach.

 From a really quick view fixing up the data xfer code paths doesn't 
look too bad.  Think I'll give it a try.

>>> Plus it doesn't do scatter-gather list handling,
>>
>> Which should be fixed anyway.
>>
> Quite. But as I said, the LSI parallel SCSI controller is going to
> suffer.

Don't think so.  Even if scsi-disk *supports* scatter lists, lsi isn't 
forced to actually use that.  I think we'll need a bounce-buffer mode 
anyway for usb-msd because it streams the scsi data in tons of small 
packets over usb ...

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Gerd Hoffmann]

   Hi,

> From a really quick view fixing up the data xfer code paths doesn't
> look too bad. Think I'll give it a try.

Oh well.  The interface pretty obviously designed for the esp, which is 
the oldest scsi adapter in qemu ...

ESP: There is no scatter-gather support in the hardware.  So for large 
reads/writes there are quite switches between OS and ESP:  The OS saying 
"dma next sectors to this location" via ioports, the ESP doing it and 
raising a IRQ when done, next round.  The existing callback mechanism 
models that pretty closely.

USB: streams the data in small packets (smaller than sector size, 64 
bytes IIRC).  Current interface works good enougth.

LSI: Hops through quite a few loops to work with the existing interface. 
  Current emulation reads one lsi script command at a time and does 
reads/writes in small pieces like the ESP.  I think it could do alot 
better: parse lsi scripts into scatter lists and submit larger requests. 
  Maybe even have multiple requests in flight at the same time.  That 
probably means putting the lsi script parsing code upside down though.

MEGASAS: I guess you have scatter lists at hand and want to submit them 
directly to the block layer for zerocopy block I/O.

So, where to go from here?

I'm tempted to zap the complete read-in-pieces logic.  For read/write 
transfers storage must be passed where everything fits in.  The 
completion callback is called on command completion and nothing else.

I think we'll need to modes here: xfer from/to host-allocated bounce 
buffer (linear buffer) and xfer from/to guest memory (scatter list).

That means (emulated) hardware without scatter-gather support must use 
the bounce buffer mode can can't do zerocopy I/O.  I don't think this is 
a big problem though.  Lots of small I/O requests don't perform very 
well, so one big request filling the bounce buffer then memcpy() from/to 
guest memory will most likely be faster anyway.

comments?
   Gerd

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Hannes Reinecke]

Am Wed 28 Oct 2009 02:58:33 PM CET schrieb Gerd Hoffmann <kraxel@redhat.com>:

>   Hi,
>
>> From a really quick view fixing up the data xfer code paths doesn't
>> look too bad. Think I'll give it a try.
>
> Oh well.  The interface pretty obviously designed for the esp, which is
> the oldest scsi adapter in qemu ...
>
> ESP: There is no scatter-gather support in the hardware.  So for large
> reads/writes there are quite switches between OS and ESP:  The OS
> saying "dma next sectors to this location" via ioports, the ESP doing
> it and raising a IRQ when done, next round.  The existing callback
> mechanism models that pretty closely.
>
> USB: streams the data in small packets (smaller than sector size, 64
> bytes IIRC).  Current interface works good enougth.
>
> LSI: Hops through quite a few loops to work with the existing
> interface.  Current emulation reads one lsi script command at a time
> and does reads/writes in small pieces like the ESP.  I think it could
> do alot better: parse lsi scripts into scatter lists and submit larger
> requests.  Maybe even have multiple requests in flight at the same
> time.  That probably means putting the lsi script parsing code upside
> down though.
>
> MEGASAS: I guess you have scatter lists at hand and want to submit them
> directly to the block layer for zerocopy block I/O.
>
Precisely.

> So, where to go from here?
>
> I'm tempted to zap the complete read-in-pieces logic.  For read/write
> transfers storage must be passed where everything fits in.  The
> completion callback is called on command completion and nothing else.
>
Agree. That would be my idea here, as well.

> I think we'll need to modes here: xfer from/to host-allocated bounce
> buffer (linear buffer) and xfer from/to guest memory (scatter list).
>
I don't think we really need two modes.
My preferred interface here is to pass down scatter-gather lists down
with every xfer; this way it'll be the responsibility of the driver to
create the lists in the first place. If it has hardware scatter-gather
support it can just pass them down, if not it can as easily create a
scatter-gather list with just one element as a bounce buffer.
Much like the current code does now, only with explicit passing of iovecs.

> That means (emulated) hardware without scatter-gather support must use
> the bounce buffer mode can can't do zerocopy I/O.  I don't think this
> is a big problem though.  Lots of small I/O requests don't perform very
> well, so one big request filling the bounce buffer then memcpy()
> from/to guest memory will most likely be faster anyway.
>
I would update the existing interface to split off the request generation from
the command completion code; then it would be easy to attach the iovec to the
request.

So something like
- Get next request
- Attach iovec/bounc-buffer
- handle request (command/write/read)
- complete request (callback)

would be an idea.

Cheers,

Hannes
---
No .sig today as I'm writing from my laptop.

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Christoph Hellwig]

On Wed, Oct 28, 2009 at 08:25:22PM +0100, Hannes Reinecke wrote:
> I don't think we really need two modes.
> My preferred interface here is to pass down scatter-gather lists down
> with every xfer; this way it'll be the responsibility of the driver to
> create the lists in the first place. If it has hardware scatter-gather
> support it can just pass them down, if not it can as easily create a
> scatter-gather list with just one element as a bounce buffer.

Yes.  If this really causes performance problems for esp we can add
bounce buffering in that driver later.

> So something like
> - Get next request
> - Attach iovec/bounc-buffer
> - handle request (command/write/read)
> - complete request (callback)

Btw, from some previuous attempts to sort out this code here are some
thing that I think would be beneficial:

 - try to create generic scsi device/request structures that the hba
   driver can access, and which contain additional private data for
   scsi-disk/generic.  Information in the generic one would include
   the information about the data transfer, the tag and the command
   block.
 - try to get rid of the tag indexing stuff by just using a pointer to
   the generic scsi request in the hba drivers.  That should get rid
   of a lot of useless list searching.

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Gerd Hoffmann]

On 10/29/09 05:37, Christoph Hellwig wrote:

>   - try to create generic scsi device/request structures that the hba
>     driver can access, and which contain additional private data for
>     scsi-disk/generic.  Information in the generic one would include
>     the information about the data transfer, the tag and the command
>     block.

Agreed.  For scsi device this did already happen as part of the 
qdev-ification work.  It certainly makes sense to do that for scsi 
requests too.  May also allow to kill some code duplication in 
scsi-disk.c and scsi-generic.c

>   - try to get rid of the tag indexing stuff by just using a pointer to
>     the generic scsi request in the hba drivers.  That should get rid
>     of a lot of useless list searching.

Indeed.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Gerd Hoffmann]

On 10/29/09 05:37, Christoph Hellwig wrote:
>> So something like
>> - Get next request
>> - Attach iovec/bounc-buffer
>> - handle request (command/write/read)
>> - complete request (callback)
>
> Btw, from some previuous attempts to sort out this code here are some
> thing that I think would be beneficial:

Trying to go forward in review+bisect friendly baby steps.  Here is what 
I have now:

http://repo.or.cz/w/qemu/kraxel.git?a=shortlog;h=refs/heads/scsi.v1

It is far from being completed, will continue tomorrow.  Should give a 
idea of the direction I'm heading to though.  Comments welcome.

/me leaves to pick up the kids,

   Gerd

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Christoph Hellwig]

On Thu, Oct 29, 2009 at 01:57:40PM +0100, Gerd Hoffmann wrote:
> Trying to go forward in review+bisect friendly baby steps.  Here is what 
> I have now:
> 
> http://repo.or.cz/w/qemu/kraxel.git?a=shortlog;h=refs/heads/scsi.v1
> 
> It is far from being completed, will continue tomorrow.  Should give a 
> idea of the direction I'm heading to though.  Comments welcome.

Nice.

I had patches for the header renames, too - the current naming is
really awkward.  Would be great if Anthony could take those ASAP.

Btw, I also splitted out a cdrom.h from the new scsi.h, not sure if it's
worth it for the two prototypes.

Just wondering, shouldn't scsi-bus.c just become scsi.c now that it's
growing various non-bus glue bits?

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Anthony Liguori]

Christoph Hellwig wrote:
> On Thu, Oct 29, 2009 at 01:57:40PM +0100, Gerd Hoffmann wrote:
>   
>> Trying to go forward in review+bisect friendly baby steps.  Here is what 
>> I have now:
>>
>> http://repo.or.cz/w/qemu/kraxel.git?a=shortlog;h=refs/heads/scsi.v1
>>
>> It is far from being completed, will continue tomorrow.  Should give a 
>> idea of the direction I'm heading to though.  Comments welcome.
>>     
>
> Nice.
>
> I had patches for the header renames, too - the current naming is
> really awkward.  Would be great if Anthony could take those ASAP.
>   

Which patches are those?

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Christoph Hellwig]

On Thu, Oct 29, 2009 at 10:14:19AM -0500, Anthony Liguori wrote:
> Which patches are those?

http://repo.or.cz/w/qemu/kraxel.git?a=commitdiff;h=1ee5ee08e4427c3db7e1322d30cc0e58e5ca48b9

and

http://repo.or.cz/w/qemu/kraxel.git?a=commitdiff;h=a6e6178185786c582141f993272e00521d3f125a

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Anthony Liguori]

Christoph Hellwig wrote:
> On Thu, Oct 29, 2009 at 10:14:19AM -0500, Anthony Liguori wrote:
>   
>> Which patches are those?
>>     
>
> http://repo.or.cz/w/qemu/kraxel.git?a=commitdiff;h=1ee5ee08e4427c3db7e1322d30cc0e58e5ca48b9
>
> and
>
> http://repo.or.cz/w/qemu/kraxel.git?a=commitdiff;h=a6e6178185786c582141f993272e00521d3f125a
>   

Doesn't look like they've gone on the list yet.

When they do, poke me and I'll make sure to process them quickly.  One 
of my short term goals is to get better at handling easy patches more 
quickly.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Gerd Hoffmann]

   Hi,

> When they do, poke me and I'll make sure to process them quickly.

Just sent.

> One of
> my short term goals is to get better at handling easy patches more quickly.

Great.  /me tests the new process ;)

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Hannes Reinecke]

Gerd Hoffmann wrote:
> On 10/29/09 05:37, Christoph Hellwig wrote:
>>> So something like
>>> - Get next request
>>> - Attach iovec/bounc-buffer
>>> - handle request (command/write/read)
>>> - complete request (callback)
>>
>> Btw, from some previuous attempts to sort out this code here are some
>> thing that I think would be beneficial:
> 
> Trying to go forward in review+bisect friendly baby steps.  Here is what
> I have now:
> 
> http://repo.or.cz/w/qemu/kraxel.git?a=shortlog;h=refs/heads/scsi.v1
> 
> It is far from being completed, will continue tomorrow.  Should give a
> idea of the direction I'm heading to though.  Comments welcome.
> 
Yep, this looks good.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Gerd Hoffmann]

On 10/30/09 09:12, Hannes Reinecke wrote:
> Gerd Hoffmann wrote:
>> http://repo.or.cz/w/qemu/kraxel.git?a=shortlog;h=refs/heads/scsi.v1
>>
>> It is far from being completed, will continue tomorrow.  Should give a
>> idea of the direction I'm heading to though.  Comments welcome.
>>
> Yep, this looks good.

More bits available now at:

http://repo.or.cz/w/qemu/kraxel.git/shortlog/refs/heads/scsi.v3

Please have a look at the new interface, this commit:

http://repo.or.cz/w/qemu/kraxel.git/commitdiff/9c825dac540282dd4d5f5f660ca13af617888037

The workflow I have in mind is:

   ->request_new()

Returns a parsed request, i.e. all fields of req->cmd are filled 
already, so the host adapter can easily check whenever it is a read or 
write and how many bytes will be transfered.

   ->request_run()

Execute the command.  iovec is passed to the dma_bdrv_*() functions, 
which means it should contain guest physical addresses.

When the request is done the complete callback is called.  For commands 
which complete immediately the callback might be called before 
request_run() returns.  Or maybe don't call the callback at all then? 
We can easily indicate using the return value that we are done already.

   ->request_del()

Release request when done.

Questions & comments are welcome.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation

[Paul Brook]

> But I certainly do _not_ want to update the SCSI disk
> emulation, as this is really quite tied to the SCSI parallel
> interface used by the old lsi53c895a.c.

This is completely untrue. scsi-disk.c contains no transport-specific code. It 
is deliberately designed to be independent of both the transport (e.g. 
parallel SCSI or USB) and the mechanism by which the initiator transfers data 
to the host.

Paul