MSI-X not enabled for ixgbe device-passthrough
Hi all, I'm trying to setup a system with device-passthrough for an ixgbe NIC. The device itself seems to work, but it isn't using MSI-X. So some more advanced features like DCB offloading etc won't work. lspci output of the device: 07:00.0 Ethernet controller: Intel Corporation 82599EB 10-Gigabit Network Connection (rev 01) Subsystem: Intel Corporation Ethernet Server Adapter X520-2 Flags: bus master, fast devsel, latency 0, IRQ 24 Memory at f5c8 (64-bit, prefetchable) [size=512K] I/O ports at 5000 [size=32] Memory at f5c7 (64-bit, prefetchable) [size=16K] [virtual] Expansion ROM at e710 [disabled] [size=512K] Capabilities: [40] Power Management version 3 Capabilities: [50] Message Signalled Interrupts: Mask+ 64bit+ Count=1/1 Enable- Capabilities: [70] MSI-X: Enable+ Mask- TabSize=64 Capabilities: [a0] Express Endpoint, MSI 00 Capabilities: [100] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq+ ACSVoil- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq+ ACSVoil- UESvrt: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSVoil- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+ CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr- AERCap: First Error Pointer: 00, GenCap+ CGenEn- ChkCap+ ChkEn- Capabilities: [140] Device Serial Number 40-9e-3c-ff-ff-21-1b-00 Capabilities: [150] Alternative Routing-ID Interpretation (ARI) ARICap: MFVC- ACS-, Next Function: 1 ARICtl: MFVC- ACS-, Function Group: 0 Capabilities: [160] Single Root I/O Virtualization (SR-IOV) IOVCap: Migration-, Interrupt Message Number: 000 IOVCtl: Enable- Migration- Interrupt- MSE- ARIHierarchy+ IOVSta: Migration- Initial VFs: 64, Total VFs: 64, Number of VFs: 64, Function Dependency Link: 00 VF offset: 128, stride: 2, Device ID: 10ed Supported Page Size: 0553, System Page Size: 0001 VF Migration: offset: , BIR: 1 Kernel driver in use: ixgbe Kernel modules: ixgbe please let me know if you need more information. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: MSI-X not enabled for ixgbe device-passthrough
Sheng Yang wrote: On Wednesday 24 March 2010 23:54:15 Hannes Reinecke wrote: Hi all, I'm trying to setup a system with device-passthrough for an ixgbe NIC. The device itself seems to work, but it isn't using MSI-X. So some more advanced features like DCB offloading etc won't work. How about lspci result in the guest? I have to boot a different system; will provide it shortly. And some guest dmesg would also help. Ok: device: 07:00.0: driver=pci-assign host=07:00.0 device: 07:00.1: driver=pci-assign host=07:00.1 [0.00] Initializing cgroup subsys cpuset [0.00] Initializing cgroup subsys cpu [0.00] Linux version 2.6.32.9-0.5-default (ge...@buildhost) (gcc version 4.3.4 [gcc-4_3-branch revision 152973] (SUSE Linux) ) #1 SMP 2010-03-15 12:22:00 +0100 [0.00] Command line: console=ttyS0 [0.00] KERNEL supported cpus: [0.00] Intel GenuineIntel [0.00] AMD AuthenticAMD [0.00] Centaur CentaurHauls [0.00] BIOS-provided physical RAM map: [0.00] BIOS-e820: - 0009f400 (usable) [0.00] BIOS-e820: 0009f400 - 000a (reserved) [0.00] BIOS-e820: 000f - 0010 (reserved) [0.00] BIOS-e820: 0010 - 07ffd000 (usable) [0.00] BIOS-e820: 07ffd000 - 0800 (reserved) [0.00] BIOS-e820: fffbc000 - 0001 (reserved) [0.00] DMI 2.4 present. [0.00] last_pfn = 0x7ffd max_arch_pfn = 0x4 [0.00] PAT not supported by CPU. [0.00] Scanning 1 areas for low memory corruption [0.00] modified physical RAM map: [0.00] modified: - 1000 (usable) [0.00] modified: 1000 - 6000 (reserved) [0.00] modified: 6000 - 0009f400 (usable) [0.00] modified: 0009f400 - 000a (reserved) [0.00] modified: 000f - 0010 (reserved) [0.00] modified: 0010 - 07ffd000 (usable) [0.00] modified: 07ffd000 - 0800 (reserved) [0.00] modified: fffbc000 - 0001 (reserved) [0.00] init_memory_mapping: -07ffd000 [0.00] RAMDISK: 07849000 - 07feff7e [0.00] ACPI: RSDP 000f8950 00014 (v00 BOCHS ) [0.00] ACPI: RSDT 07ffde30 00034 (v01 BOCHS BXPCRSDT 0001 BXPC 0001) [0.00] ACPI: FACP 07fffe70 00074 (v01 BOCHS BXPCFACP 0001 BXPC 0001) [0.00] ACPI: DSDT 07ffdfd0 01E22 (v01 BXPC BXDSDT 0001 INTL 20090123) [0.00] ACPI: FACS 07fffe00 00040 [0.00] ACPI: SSDT 07ffdf90 00037 (v01 BOCHS BXPCSSDT 0001 BXPC 0001) [0.00] ACPI: APIC 07ffdeb0 00072 (v01 BOCHS BXPCAPIC 0001 BXPC 0001) [0.00] ACPI: HPET 07ffde70 00038 (v01 BOCHS BXPCHPET 0001 BXPC 0001) [0.00] No NUMA configuration found [0.00] Faking a node at -07ffd000 [0.00] Bootmem setup node 0 -07ffd000 [0.00] NODE_DATA [9000 - 0003cfff] [0.00] bootmap [0003d000 - 0003dfff] pages 1 [0.00] (7 early reservations) == bootmem [00 - 0007ffd000] [0.00] #0 [00 - 001000] BIOS data page == [00 - 001000] [0.00] #1 [006000 - 008000] TRAMPOLINE == [006000 - 008000] [0.00] #2 [000100 - 0001cd6778]TEXT DATA BSS == [000100 - 0001cd6778] [0.00] #3 [0007849000 - 0007feff7e] RAMDISK == [0007849000 - 0007feff7e] [0.00] #4 [09f400 - 10]BIOS reserved == [09f400 - 10] [0.00] #5 [0001cd7000 - 0001cd7049] BRK == [0001cd7000 - 0001cd7049] [0.00] #6 [008000 - 009000] PGTABLE == [008000 - 009000] [0.00] found SMP MP-table at [880f89a0] f89a0 [0.00] kvm-clock: cpu 0, msr 0:1940501, boot clock [0.00] Zone PFN ranges: [0.00] DMA 0x - 0x1000 [0.00] DMA320x1000 - 0x0010 [0.00] Normal 0x0010 - 0x0010 [0.00] Movable zone start PFN for each node [0.00] early_node_map[3] active PFN ranges [0.00] 0: 0x - 0x0001 [0.00] 0: 0x0006 - 0x009f [0.00] 0: 0x0100 - 0x7ffd [0.00] ACPI: PM-Timer IO Port: 0xb008 [0.00] ACPI: LAPIC (acpi_id[0x00] lapic_id[0x00] enabled) [0.00] ACPI: IOAPIC (id[0x01] address[0xfec0] gsi_base[0]) [0.00
Re: MSI-X not enabled for ixgbe device-passthrough
Chris Wright wrote: * Hannes Reinecke (h...@suse.de) wrote: Hi all, I'm trying to setup a system with device-passthrough for an ixgbe NIC. The device itself seems to work, but it isn't using MSI-X. So some more advanced features like DCB offloading etc won't work. Please send the relevant dmesg from the guest when it initializes the device. BTW, more typical case for that NIC is to assign the VF to the guest, not the whole PF. Yes, I know. But my kernel I'm testing with doesn't have one for ixgbe. So I tested that one. And KVM really should enable MSI-X here, VFs notwithstanding. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: MSI-X not enabled for ixgbe device-passthrough
Chris Wright wrote: * Hannes Reinecke (h...@suse.de) wrote: Chris Wright wrote: * Hannes Reinecke (h...@suse.de) wrote: Hi all, I'm trying to setup a system with device-passthrough for an ixgbe NIC. The device itself seems to work, but it isn't using MSI-X. So some more advanced features like DCB offloading etc won't work. Please send the relevant dmesg from the guest when it initializes the device. BTW, more typical case for that NIC is to assign the VF to the guest, not the whole PF. Yes, I know. But my kernel I'm testing with doesn't have one for ixgbe. Ah, you mean it's an older (heh, ok, older actually means not brand new upstream) kernel, w/out the recent sr-iov additions, fair enough. So I tested that one. And KVM really should enable MSI-X here, VFs notwithstanding. Yeah, although it's not just KVM involved, it's very much driven by the guest too. The guest will see (or at least should see) the MSI-X capability, and decide based on the number of queues whether to enable MSI-X (completely driver dependent here). Did you have a chance to boot the guest again, and send the lspci -vvv from the guest POV? You should see two PCI capabilities (MSI and 0x40 and MSI-X at 0x50). Actually, I have one of these devices, let me give it a quick test... working fine here. Here's some relevant information: Host: $ sudo lspci -v -s 04:00.0 04:00.0 Ethernet controller: Intel Corporation 82599EB 10-Gigabit Network Connection (rev 01) Subsystem: Intel Corporation Ethernet Server Adapter X520-2 Flags: bus master, fast devsel, latency 0, IRQ 16 Memory at f808 (64-bit, prefetchable) [size=512K] I/O ports at d020 [size=32] Memory at f8104000 (64-bit, prefetchable) [size=16K] Capabilities: [40] Power Management version 3 Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+ Capabilities: [70] MSI-X: Enable+ Count=64 Masked- Capabilities: [a0] Express Endpoint, MSI 00 Capabilities: [100] Advanced Error Reporting Capabilities: [140] Device Serial Number 00-1b-21-ff-ff-3f-a1-b0 Capabilities: [150] Alternative Routing-ID Interpretation (ARI) Capabilities: [160] Single Root I/O Virtualization (SR-IOV) $ grep kvm /proc/interrupts 95:289 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 PCI-MSI-edge kvm_assigned_msix_device 96: 32 0 0 0292 0 0 0 0 0 0 0 0 0 0 0 PCI-MSI-edge kvm_assigned_msix_device 97: 2 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 PCI-MSI-edge kvm_assigned_msix_device Guest side: $ sudo lspci -vvv -s 00:04.0 00:04.0 Ethernet controller: Intel Corporation 82599EB 10-Gigabit Network Connection (rev 01) Subsystem: Intel Corporation Ethernet Server Adapter X520-2 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast TAbort- TAbort- MAbort- SERR- PERR- INTx- Latency: 0, Cache Line Size: 64 bytes Interrupt: pin A routed to IRQ 11 Region 0: Memory at f208 (32-bit, non-prefetchable) Region 2: I/O ports at c200 Region 4: Memory at f210 (32-bit, non-prefetchable) Capabilities: [40] MSI: Enable- Count=1/1 Maskable- 64bit- Address: Data: Capabilities: [50] MSI-X: Enable+ Count=64 Masked- Vector table: BAR=4 offset= PBA: BAR=4 offset=2000 $ grep eth1 /proc/interrupts 177:387 PCI-MSI-X eth1-rx-0 185:421 PCI-MSI-X eth1-tx-0 193: 2 PCI-MSI-X eth1:lsc $ dmesg | grep -e 00:04.0 -e ixgbe ixgbe: Intel(R) 10 Gigabit PCI Express Network Driver - version 2.0.8-k2 ixgbe: Copyright (c) 1999-2009 Intel Corporation. ACPI: PCI Interrupt :00:04.0[A] - Link [LNKD] - GSI 10 (level, high) - IRQ 10 PCI: Setting latency timer of device :00:04.0 to 64 ixgbe: :00:04.0: ixgbe_init_interrupt_scheme: Multiqueue Disabled: Rx Queue count = 1, Tx Queue count = 1 ixgbe :00:04.0: (PCI Express:2.5Gb/s:Width x8) 00:1b:21:3f:a1:b0 ixgbe :00:04.0: MAC: 2, PHY: 11, SFP+: 5, PBA No: e66562-003 ixgbe :00:04.0: Intel(R) 10 Gigabit Network Connection ixgbe: eth1 NIC Link is Up 10 Gbps, Flow Control: None Ah. So I'll have to shout at Alex Graf. No problems there :-) Thanks for your help. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX
Re: [QEMU-KVM]: Megasas + TCM_Loop + SG_IO into Windows XP guests
Nicholas A. Bellinger wrote:
Greetings Hannes and co,
I have been spending a bit of time trying Megasas HBA emulation +
TCM_Loop + SG_IO with a Windows XP SP2 KVM guests.. So far, I noticed
that hw/scsi-generic.c:execute_command_run() using bdev_aio_ioctl()
appears to be broken for XP guests, which causes the first 36-byte
INQUIRY sent via SG_IO to never make it back to QEMU and results in the
win32 LSI drive taking the LUN offline, et al. Note that everything
does appear to be functioning as expected in kernel space for the first
INQUIRY with the TCM_Loop LLD and Linux/SCSI code (AFAICT) and Linux KVM
guests using megasas emulation are still working.
Now that is really odd. Have you checked if it works with the
'normal' KVM disk backend?
So, I ended up needing requiring the following quick hack for
hw/scsi-generic.c:execute_command_run() to make SG_IO function
synchronously using bdrv_ioctl(), which at least gets LUN registration
and basic control path CDBs working for the XP guest.
Here is how it looks in action on a v2.6.34-rc7 host so far:
http://www.linux-iscsi.org/images/TCM-KVM-megasas-XP-05132010.png
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 6c58742..aa1eb83 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -140,6 +140,7 @@ static int execute_command_run(SCSIGenericReq *r,
{
BlockDriverState *bdrv = r-req.dev-conf.dinfo-bdrv;
SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, r-req.dev);
+int ret;
r-io_header.interface_id = 'S';
r-io_header.dxfer_direction = sgdir[r-req.cmd.mode];
@@ -161,11 +162,16 @@ static int execute_command_run(SCSIGenericReq *r,
printf(\n);
}
#endif
+#if 0
r-req.aiocb = bdrv_aio_ioctl(bdrv, SG_IO, r-io_header, complete, r);
if (r-req.aiocb == NULL) {
BADF(execute_command: read failed !\n);
return -1;
}
+#else
+ret = bdrv_ioctl(bdrv, SG_IO, r-io_header);
+complete((void *)r, ret);
+#endif
* return 0;
}
Beyond the initial LUN registration and control CDB parts, doing bulk
DATA_SG_IO traffic is completing successfully (and everything looks sane
with TCM_Loop and Linux/SCSI) but it appears that the correct blocks are
not actually getting written/read by megasas. This appears to be the
case with both hw/scsi-generic.c and hw/scsi-disk.c modes of operation
for megasas with the win32 XP guest.
Oh. Hmm.
So I was wondering if anyone aware of known issues with QEMU
asynchronous SG_IO into MSFT KVM guests with virtio or hw/lsi53c895a.c,
or would this be something specific to megasas HBA emulation and XP
guests..?
Hannes, which MSFT guest + driver did you get work stable with bulk
DATA_SG_IO and hw/scsi-disk.c..?
Well, I have two more patches for megasas.
The one is just a cleanup to remove duplicate definitions, but the
other contains a real issue with a misjudged cast in megasas_enqueue_frame().
Not sure if that helps here, but it's worth a try nevertheless.
I'll be sending them with separate mails.
Let's see if I can find some time working on the megasas emulation.
Maybe I find something.
Last time I checked it was with a Windows7 build, but I didn't do
any real tests there. Basically just checking if the system boots up :-)
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [QEMU-KVM]: Megasas + TCM_Loop + SG_IO into Windows XP guests
Nicholas A. Bellinger wrote: On Fri, 2010-05-14 at 02:42 -0700, Nicholas A. Bellinger wrote: On Fri, 2010-05-14 at 09:22 +0200, Hannes Reinecke wrote: Nicholas A. Bellinger wrote: Greetings Hannes and co, SNIP Let's see if I can find some time working on the megasas emulation. Maybe I find something. Last time I checked it was with a Windows7 build, but I didn't do any real tests there. Basically just checking if the system boots up :-) Nothing fancy just yet. This is involving a normal NTFS filesystem format on a small TCM/FILEIO LUN using scsi-generic and a userspace FILEIO with scsi-disk. This involves the XP guest waiting until the very last READ_10 once the format has completed (eg: all WRITE and VERIFY CDBs complete with GOOD status AFAICT) before announcing that mkfs.ntfs failed without any helpful exception message (due to missing metadata of some sort I would assume..?) So perhaps dumping QEMU and TCM_Loop SCSI payloads to determine if any correct blocks from megasas_handle_io() are actually making it out to KVM host is going to be my next option. ;) Greetings Hannes, So I spent some more time with XP guests this weekend, and I noticed two things immediately when using hw/lsi53c895a.c instead of hw/megasas.c with the same two TCM_Loop SAS LUNs via SG_IO from last week: 1) With lsi53c895a, XP guests are able to boot successfully w/ out the synchronous SG_IO hack that is currently required to get past the first 36-byte INQUIRY for megasas + XP SP2 2) With lsi53c895a, XP is able to successfully create and mount a NTFS filesystem, reboot, and read blocks appear to be functioning properly. FYI I have not run any 'write known pattern then read-back and compare blocks' data integrity tests from with in the XP guests just yet, but I am confident that TCM scatterlist - se_mem_t mapping is working as expected on the KVM Host. Futhermore, after formatting a 5 GB TCM/FILEIO LUN with lsi53c895a, and then rebooting with megasas with the same two configured TCM_Loop SG_IO devices, it appears to be able to mount and read blocks successfully. Attempting to write new blocks on the mounted filesystem also appears to work to some degree, but throughput slows down to a crawl during XP guest buffer cache flush, which is likely attributed to the use of my quick SYNC SG_IO hack. So it appears that there are two seperate issues here, and AFAICT they both look to be XP and megasas specific. For #2, it may be something about the format of the incoming scatterlists generated during XP's mkfs.ntfs that is causing some issues. While watching output during fs creation, I noticed the following WRITE_10s with a starting 4088 byte scatterlist and a trailing 8 byte scatterlist: megasas: writel mmio 40: 2b0b003 megasas: Found mapped frame 2 context 82b0b000 pa 2b0b000 megasas: Enqueue frame context 82b0b000 tail 493 busy 1 megasas: LD SCSI dev 2 lun 0 sdev 0xdc0230 xfer 16384 scsi-generic: Using cur_addr: 0x0ff6c008 cur_len: 0x0ff8 scsi-generic: Adding iovec for mem: 0x7f1783b96008 len: 0x0ff8 scsi-generic: Using cur_addr: 0x0fd6e000 cur_len: 0x1000 scsi-generic: Adding iovec for mem: 0x7f1783998000 len: 0x1000 scsi-generic: Using cur_addr: 0x0fe2f000 cur_len: 0x1000 scsi-generic: Adding iovec for mem: 0x7f1783a59000 len: 0x1000 scsi-generic: Using cur_addr: 0x0fdf cur_len: 0x1000 scsi-generic: Adding iovec for mem: 0x7f1783a1a000 len: 0x1000 scsi-generic: Using cur_addr: 0x0fded000 cur_len: 0x0008 scsi-generic: Adding iovec for mem: 0x7f1783a17000 len: 0x0008 scsi-generic: execute IOV: iovec_count: 5, dxferp: 0xd92420, dxfer_len: 16384 scsi-generic: --- Issuing SG_IO CDB len 10: 0x2a 00 00 00 fa be 00 00 20 00 scsi-generic: scsi_write_complete() ret = 0 scsi-generic: Command complete 0x0xd922c0 tag=0x82b0b000 status=0 megasas: LD SCSI req 0xd922c0 cmd 0xda92c0 lun 0xdc0230 finished with status 0 len 16384 megasas: Complete frame context 82b0b000 tail 493 busy 0 doorbell 0 Also, the final READ_10 that produces the 'could not create filesystem' exception is for LBA 63 and XP looking for the first FS blocks after GPT. Could there be some breakage in megasas with a length PAGE_SIZE for the scatterlist..?As lsi53c895a seems to work OK for this case, is there something about the logic of parsing the incoming struct scatterlists that is different between the two HBA drivers..? AFAICT both are using Gerd's common code in hw/scsi-bus.c, unless there is something about megasas_map_sgl() that is causing issues with the above..? The usual disclaimer here: I'm less than happy with the current SCSI disk handling. Currently we have the two options: - Using 'scsi-disk', which will _emulate_ a SCSI disk internally, but allow to use asynchronous I/O
[PATCH 0/4] megaraid_sas HBA emulation
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 h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] Add 'raid' interface class
This patch adds a 'raid' interface class. It is basically a clone
of the existing 'scsi' interface, only allowing up to 128 disks.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/pc.c |5 +
hw/pci-hotplug.c |1 +
hw/scsi-disk.c | 17 +
hw/scsi-disk.h | 20 +++-
qemu-config.c|2 +-
sysemu.h |3 ++-
vl.c |8 ++--
7 files changed, 51 insertions(+), 5 deletions(-)
diff --git a/hw/pc.c b/hw/pc.c
index 83012a9..26aad4c 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1345,6 +1345,11 @@ static void pc_init1(ram_addr_t ram_size,
for (bus = 0; bus = max_bus; bus++) {
pci_create_simple(pci_bus, -1, lsi53c895a);
}
+
+ max_bus = drive_get_max_bus(IF_RAID);
+ for (bus = 0; bus = max_bus; bus++) {
+ pci_create_simple(pci_bus, -1, megasas);
+ }
}
if (extboot_drive) {
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 410fa3f..855a1ad 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -85,6 +85,7 @@ void drive_hot_add(Monitor *mon, const QDict *qdict)
switch (type) {
case IF_SCSI:
+case IF_RAID:
if (pci_read_devaddr(mon, pci_addr, dom, pci_bus, slot)) {
goto err;
}
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 2a9268a..68b4e83 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -41,6 +41,7 @@ do { fprintf(stderr, scsi-disk: fmt , ## __VA_ARGS__); }
while (0)
#define SCSI_DMA_BUF_SIZE131072
#define SCSI_MAX_INQUIRY_LEN 256
+#define SCSI_SENSE_LEN 18
#define SCSI_REQ_STATUS_RETRY 0x01
@@ -136,6 +137,22 @@ static SCSIRequest *scsi_find_request(SCSIDiskState *s,
uint32_t tag)
return r;
}
+/* Helper function to build a sense block */
+int32_t scsi_build_sense(uint8_t *sense_buf, uint32_t sense)
+{
+memset(sense_buf, 0, SCSI_SENSE_LEN);
+if (!sense)
+ return 0;
+
+sense_buf[0] = 0xf0; /* current, fixed format */
+sense_buf[2] = (sense 16) 0x0F;
+sense_buf[7] = 10;
+sense_buf[12] = (sense 8 ) 0xFF;
+sense_buf[13] = sense 0xFF;
+
+return SCSI_SENSE_LEN;
+}
+
/* Helper function for command completion. */
static void scsi_command_complete(SCSIRequest *r, int status, int sense)
{
diff --git a/hw/scsi-disk.h b/hw/scsi-disk.h
index b6b6c12..5b54272 100644
--- a/hw/scsi-disk.h
+++ b/hw/scsi-disk.h
@@ -9,6 +9,23 @@ enum scsi_reason {
SCSI_REASON_DATA /* Transfer complete, more data required. */
};
+/* LUN not ready, Manual intervention required */
+#define SENSE_LUN_NOT_READY 0x020403
+/* Hardware error, I/O process terminated */
+#define SENSE_IO_ERROR 0x040006
+/* Hardware error, I_T Nexus loss occured */
+#define SENSE_TAG_NOT_FOUND 0x042907
+/* Hardware error, internal target failure */
+#define SENSE_TARGET_FAILURE 0x044400
+/* Illegal request, invalid command operation code */
+#define SENSE_INVALID_OPCODE 0x052000
+/* Illegal request, LBA out of range */
+#define SENSE_LBA_OUT_OF_RANGE 0x052100
+/* Illegal request, Invalid field in CDB */
+#define SENSE_INVALID_FIELD 0x052400
+/* Illegal request, LUN not supported */
+#define SENSE_LUN_NOT_SUPPORTED 0x052500
+
typedef struct SCSIBus SCSIBus;
typedef struct SCSIDevice SCSIDevice;
typedef struct SCSIDeviceInfo SCSIDeviceInfo;
@@ -49,7 +66,7 @@ struct SCSIBus {
int tcq, ndev;
scsi_completionfn complete;
-SCSIDevice *devs[8];
+SCSIDevice *devs[128];
};
void scsi_bus_new(SCSIBus *bus, DeviceState *host, int tcq, int ndev,
@@ -63,5 +80,6 @@ static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, DriveInfo *dinfo, int
unit);
void scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
+int32_t scsi_build_sense(uint8_t *sense_buf, uint32_t sense);
#endif
diff --git a/qemu-config.c b/qemu-config.c
index 4fb7898..8d7a137 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -18,7 +18,7 @@ QemuOptsList qemu_drive_opts = {
},{
.name = if,
.type = QEMU_OPT_STRING,
-.help = interface (ide, scsi, sd, mtd, floppy, pflash, virtio),
+.help = interface (ide, scsi, raid, sd, mtd, floppy, pflash,
virtio),
},{
.name = index,
.type = QEMU_OPT_NUMBER,
diff --git a/sysemu.h b/sysemu.h
index 2ef3797..8ed0b8c 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -159,7 +159,7 @@ extern unsigned int nb_prom_envs;
typedef enum {
IF_NONE,
-IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
+IF_IDE, IF_SCSI, IF_RAID, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO,
IF_XEN,
IF_COUNT
} BlockInterfaceType;
@@ -185,6 +185,7 @@ typedef struct DriveInfo {
#define MAX_IDE_DEVS 2
#define MAX_SCSI_DEVS 7
+#define MAX_RAID_DEVS 128
#define MAX_DRIVES 32
extern QTAILQ_HEAD(drivelist, DriveInfo) drives;
diff --git a/vl.c b/vl.c
index 5dc7b2b..404afc3 100644
--- a/vl.c
+++ b/vl.c
@@ -2065,6 +2065,9
[PATCH 2/4] megasas: LSI MegaRAID SAS HBA emulation
This patch add an emulation for the LSI MegaRAID SAS HBA. It is
using SG_IO to forward / pass through SCSI commands to the
underlying block driver, so no emulation is done currently.
Signed-off-by: Hannes Reinecke h...@suse.de
---
Makefile.hw |2 +-
hw/megasas.c | 1134 ++
hw/pci_ids.h |2 +
3 files changed, 1137 insertions(+), 1 deletions(-)
create mode 100644 hw/megasas.c
diff --git a/Makefile.hw b/Makefile.hw
index de1db31..cae35f9 100644
--- a/Makefile.hw
+++ b/Makefile.hw
@@ -33,7 +33,7 @@ obj-y += wdt_i6300esb.o
obj-y += ne2000.o
# SCSI layer
-obj-y += lsi53c895a.o
+obj-y += lsi53c895a.o megasas.o
obj-$(CONFIG_ESP) += esp.o
obj-y += dma-helpers.o sysbus.o isa-bus.o
diff --git a/hw/megasas.c b/hw/megasas.c
new file mode 100644
index 000..a57e8e0
--- /dev/null
+++ b/hw/megasas.c
@@ -0,0 +1,1134 @@
+/*
+ * QEMU MegaRAID SAS 8708EM2 Host Bus Adapter emulation
+ *
+ * Copyright (c) 2009 Hannes Reinecke, SUSE Linux Products GmbH
+ *
+ * This code is licenced under the LGPL.
+ */
+
+
+#include assert.h
+
+#include hw.h
+#include pci.h
+#include scsi.h
+#include scsi-disk.h
+#include block_int.h
+#ifdef __linux__
+# include scsi/sg.h
+#endif
+
+#undef DEBUG_MEGASAS
+#undef DEBUG_MEGASAS_REG
+#undef DEBUG_MEGASAS_QUEUE
+#undef DEBUG_MEGASAS_MFI
+
+#ifdef DEBUG_MEGASAS
+#define DPRINTF(fmt, ...) \
+do { printf(megasas: fmt , ## __VA_ARGS__); } while (0)
+#define BADF(fmt, ...) \
+do { fprintf(stderr, megasas: error: fmt , ## __VA_ARGS__); exit(1);} while
(0)
+#else
+#define DPRINTF(fmt, ...) do {} while(0)
+#define BADF(fmt, ...) \
+do { fprintf(stderr, megasas: error: fmt , ## __VA_ARGS__);} while (0)
+#endif
+
+/* Static definitions */
+#define MEGASAS_MAX_FRAMES 64
+#define MEGASAS_MAX_SGE 8
+
+/* SCSI definitions */
+#define SAM_STAT_GOOD0x00
+#define SAM_STAT_CHECK_CONDITION 0x02
+#define SAM_STAT_CONDITION_MET 0x04
+#define SAM_STAT_BUSY0x08
+#define SAM_STAT_TASK_ABORTED0x40
+
+/* Register definitions */
+#defineMEGASAS_INBOUND_MSG_0 0x0010
+#defineMEGASAS_INBOUND_MSG_1 0x0014
+#defineMEGASAS_OUTBOUND_MSG_0 0x0018
+#defineMEGASAS_OUTBOUND_MSG_1 0x001C
+#defineMEGASAS_INBOUND_DOORBELL0x0020
+#defineMEGASAS_INBOUND_INTR_STATUS 0x0024
+#defineMEGASAS_INBOUND_INTR_MASK 0x0028
+#defineMEGASAS_OUTBOUND_DOORBELL 0x002C
+#defineMEGASAS_OUTBOUND_INTR_STATUS0x0030
+#defineMEGASAS_OUTBOUND_INTR_MASK 0x0034
+#defineMEGASAS_INBOUND_QUEUE_PORT 0x0040
+#defineMEGASAS_OUTBOUND_QUEUE_PORT 0x0044
+#defineMEGASAS_OUTBOUND_DOORBELL_CLEAR 0x00A0
+#defineMEGASAS_OUTBOUND_SCRATCH_PAD0x00B0
+#defineMEGASAS_INBOUND_LOW_QUEUE_PORT 0x00C0
+#defineMEGASAS_INBOUND_HIGH_QUEUE_PORT 0x00C4
+
+/* FW commands */
+#define MFI_INIT_ABORT 0x0001
+#define MFI_INIT_READY 0x0002
+#define MFI_INIT_MFIMODE 0x0004
+#define MFI_INIT_CLEAR_HANDSHAKE 0x0008
+#define MFI_INIT_HOTPLUG 0x0010
+#define MFI_STOP_ADP 0x0020
+
+/* MFI states */
+#define MFI_STATE_UNDEFINED0x0
+#define MFI_STATE_BB_INIT 0x1
+#define MFI_STATE_FW_INIT 0x4
+#define MFI_STATE_WAIT_HANDSHAKE 0x6
+#define MFI_STATE_FW_INIT_20x7
+#define MFI_STATE_DEVICE_SCAN 0x8
+#define MFI_STATE_BOOT_MESSAGE_PENDING 0x9
+#define MFI_STATE_FLUSH_CACHE 0xA
+#define MFI_STATE_READY0xB
+#define MFI_STATE_OPERATIONAL 0xC
+#define MFI_STATE_FAULT0xF
+
+/*
+ * MFI command opcodes
+ */
+#define MFI_CMD_INIT 0x00
+#define MFI_CMD_LD_READ0x01
+#define MFI_CMD_LD_WRITE 0x02
+#define MFI_CMD_LD_SCSI_IO 0x03
+#define MFI_CMD_PD_SCSI_IO 0x04
+#define MFI_CMD_DCMD 0x05
+#define MFI_CMD_ABORT 0x06
+#define MFI_CMD_SMP0x07
+#define MFI_CMD_STP0x08
+
+#define MR_DCMD_CTRL_GET_INFO 0x0101
+
+#define MR_DCMD_CTRL_CACHE_FLUSH 0x01101000
+#define MR_FLUSH_CTRL_CACHE0x01
+#define MR_FLUSH_DISK_CACHE0x02
+
+#define MR_DCMD_CTRL_SHUTDOWN 0x0105
+#define MR_DCMD_HIBERNATE_SHUTDOWN 0x0106
+#define MR_ENABLE_DRIVE_SPINDOWN 0x01
+
+#define
[PATCH 3/4] scsi-disk: Factor out SCSI command emulation
Other drives might want to use SCSI command emulation without
going through the SCSI disk abstraction, as this imposes too
many limits on the emulation.
Signed-off-by: Hannes Reinecke h...@suse.de
---
block.c| 15 ++
block.h|3 +
block_int.h|1 +
hw/scsi-disk.c | 610 ++--
hw/scsi-disk.h |3 +
5 files changed, 346 insertions(+), 286 deletions(-)
diff --git a/block.c b/block.c
index 33f3d65..06f92c4 100644
--- a/block.c
+++ b/block.c
@@ -930,6 +930,21 @@ int bdrv_is_sg(BlockDriverState *bs)
return bs-sg;
}
+void bdrv_set_sg(BlockDriverState *bs, int set)
+{
+bs-sg = set;
+}
+
+int bdrv_get_tcq(BlockDriverState *bs)
+{
+return bs-tcq;
+}
+
+void bdrv_set_tcq(BlockDriverState *bs, int set)
+{
+bs-tcq = set;
+}
+
int bdrv_enable_write_cache(BlockDriverState *bs)
{
return bs-enable_write_cache;
diff --git a/block.h b/block.h
index a966afb..7862fa0 100644
--- a/block.h
+++ b/block.h
@@ -134,9 +134,12 @@ void bdrv_get_geometry_hint(BlockDriverState *bs,
int *pcyls, int *pheads, int *psecs);
int bdrv_get_type_hint(BlockDriverState *bs);
int bdrv_get_translation_hint(BlockDriverState *bs);
+int bdrv_get_tcq(BlockDriverState *bs);
+void bdrv_set_tcq(BlockDriverState *bs, int set);
int bdrv_is_removable(BlockDriverState *bs);
int bdrv_is_read_only(BlockDriverState *bs);
int bdrv_is_sg(BlockDriverState *bs);
+void bdrv_set_sg(BlockDriverState *bs, int set);
int bdrv_enable_write_cache(BlockDriverState *bs);
int bdrv_is_inserted(BlockDriverState *bs);
int bdrv_media_changed(BlockDriverState *bs);
diff --git a/block_int.h b/block_int.h
index 8e72abe..e5ee57b 100644
--- a/block_int.h
+++ b/block_int.h
@@ -129,6 +129,7 @@ struct BlockDriverState {
int encrypted; /* if true, the media is encrypted */
int valid_key; /* if true, a valid encryption key has been set */
int sg;/* if true, the device is a /dev/sg* */
+int tcq; /* if true, the device supports tagged command queueing */
/* event callback when inserting/removing */
void (*change_cb)(void *opaque);
void *change_opaque;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 68b4e83..3e68518 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -56,7 +56,7 @@ typedef struct SCSIRequest {
/* Both sector and sector_count are in terms of qemu 512 byte blocks. */
uint64_t sector;
uint32_t sector_count;
-struct iovec iov;
+struct iovec *iov;
QEMUIOVector qiov;
BlockDriverAIOCB *aiocb;
struct SCSIRequest *next;
@@ -72,7 +72,8 @@ struct SCSIDiskState
This is the number of 512 byte blocks in a single scsi sector. */
int cluster_size;
uint64_t max_lba;
-int sense;
+uint8_t sense[SCSI_SENSE_LEN];
+uint8_t sense_len;
char drive_serial_str[21];
QEMUBH *bh;
};
@@ -90,13 +91,12 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d,
uint32_t tag)
free_requests = r-next;
} else {
r = qemu_malloc(sizeof(SCSIRequest));
-r-iov.iov_base = qemu_memalign(512, SCSI_DMA_BUF_SIZE);
+ r-iov = NULL;
}
r-bus = scsi_bus_from_device(d);
r-dev = s;
r-tag = tag;
r-sector_count = 0;
-r-iov.iov_len = 0;
r-aiocb = NULL;
r-status = 0;
@@ -126,6 +126,17 @@ static void scsi_remove_request(SCSIRequest *r)
free_requests = r;
}
+static void *scsi_allocate_iovec(SCSIRequest *r) {
+if (!r-iov) {
+ r-iov = qemu_malloc(sizeof(struct iovec));
+ if (!r-iov)
+ return NULL;
+ r-iov-iov_base = qemu_memalign(512, SCSI_DMA_BUF_SIZE);
+ r-iov-iov_len = SCSI_DMA_BUF_SIZE;
+}
+return r-iov;
+}
+
static SCSIRequest *scsi_find_request(SCSIDiskState *s, uint32_t tag)
{
SCSIRequest *r;
@@ -137,12 +148,11 @@ static SCSIRequest *scsi_find_request(SCSIDiskState *s,
uint32_t tag)
return r;
}
-/* Helper function to build a sense block */
int32_t scsi_build_sense(uint8_t *sense_buf, uint32_t sense)
{
memset(sense_buf, 0, SCSI_SENSE_LEN);
if (!sense)
- return 0;
+ return 0;
sense_buf[0] = 0xf0; /* current, fixed format */
sense_buf[2] = (sense 16) 0x0F;
@@ -154,15 +164,19 @@ int32_t scsi_build_sense(uint8_t *sense_buf, uint32_t
sense)
}
/* Helper function for command completion. */
-static void scsi_command_complete(SCSIRequest *r, int status, int sense)
+static void scsi_command_complete(SCSIRequest *r, int status, uint32_t sense)
{
SCSIDiskState *s = r-dev;
uint32_t tag;
-DPRINTF(Command complete tag=0x%x status=%d sense=%d\n, r-tag, status,
sense);
-s-sense = sense;
+
+DPRINTF(Command complete tag=0x%x status=%d sense=%d\n, r-tag,
+ status, s-sense_len);
+if (status == STATUS_CHECK_CONDITION) {
+ s-sense_len = scsi_build_sense(s-sense, sense);
+}
tag = r-tag;
scsi_remove_request(r);
-r-bus-complete(r-bus
[PATCH 4/4] megasas: Add SCSI command emulation
Now that we can use SCSI command emulation without using the SCSI
disk abstraction we can easily add it to the megasas HBA.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/megasas.c | 88 +++---
1 files changed, 53 insertions(+), 35 deletions(-)
diff --git a/hw/megasas.c b/hw/megasas.c
index a57e8e0..f32b313 100644
--- a/hw/megasas.c
+++ b/hw/megasas.c
@@ -661,40 +661,55 @@ static int megasas_handle_scsi(MPTState *s, uint8_t fcmd,
}
}
-memset(cmd-hdr, 0, sizeof(struct sg_io_hdr));
-cmd-hdr.interface_id = 'S';
-cmd-hdr.cmd_len = cdb_len;
-cmd-hdr.cmdp = cdb;
-cmd-hdr.iovec_count = cmd-sge_count;
-cmd-hdr.dxferp = cmd-iov;
-for (n = 0; n cmd-sge_count; n++)
- cmd-hdr.dxfer_len += cmd-iov[n].iov_len;
-if (cmd-sge_count) {
- if (dir)
- cmd-hdr.dxfer_direction = SG_DXFER_TO_DEV;
- else
- cmd-hdr.dxfer_direction = SG_DXFER_FROM_DEV;
-} else {
- cmd-hdr.dxfer_direction = SG_DXFER_NONE;
-}
-cmd-hdr.sbp = cmd-sense;
-cmd-hdr.mx_sb_len = cmd-sense_len;
+if (bdrv_is_sg(cmd-lun-bdrv)) {
+ memset(cmd-hdr, 0, sizeof(struct sg_io_hdr));
+ cmd-hdr.interface_id = 'S';
+ cmd-hdr.cmd_len = cdb_len;
+ cmd-hdr.cmdp = cdb;
+ cmd-hdr.iovec_count = cmd-sge_count;
+ cmd-hdr.dxferp = cmd-iov;
+ for (n = 0; n cmd-sge_count; n++)
+ cmd-hdr.dxfer_len += cmd-iov[n].iov_len;
+ if (cmd-sge_count) {
+ if (dir)
+ cmd-hdr.dxfer_direction = SG_DXFER_TO_DEV;
+ else
+ cmd-hdr.dxfer_direction = SG_DXFER_FROM_DEV;
+ } else {
+ cmd-hdr.dxfer_direction = SG_DXFER_NONE;
+ }
+ cmd-hdr.sbp = cmd-sense;
+ cmd-hdr.mx_sb_len = cmd-sense_len;
-ret = bdrv_ioctl(cmd-lun-bdrv, SG_IO, cmd-hdr);
-if (ret) {
- DPRINTF(SCSI pthru dev %x lun %x failed with %d\n,
- target, lun, errno);
- sense_len = scsi_build_sense(cmd-sense, SENSE_IO_ERROR);
- cmd-sge_size = 0;
- scsi_status = SAM_STAT_CHECK_CONDITION;
-} else if (cmd-hdr.status) {
- sense_len = cmd-hdr.sb_len_wr;
- scsi_status = cmd-hdr.status;
- cmd-sge_size = cmd-hdr.dxfer_len;
- scsi_status = SAM_STAT_CHECK_CONDITION;
+ ret = bdrv_ioctl(cmd-lun-bdrv, SG_IO, cmd-hdr);
+ if (ret) {
+ DPRINTF(SCSI pthru dev %x lun %x failed with %d\n,
+ target, lun, errno);
+ sense_len = scsi_build_sense(cmd-sense, SENSE_IO_ERROR);
+ cmd-sge_size = 0;
+ scsi_status = SAM_STAT_CHECK_CONDITION;
+ } else if (cmd-hdr.status) {
+ sense_len = cmd-hdr.sb_len_wr;
+ scsi_status = cmd-hdr.status;
+ cmd-sge_size = cmd-hdr.dxfer_len;
+ scsi_status = SAM_STAT_CHECK_CONDITION;
+ } else {
+ sense_len = 0;
+ cmd-sge_size = cmd-hdr.dxfer_len;
+ }
} else {
- sense_len = 0;
- cmd-sge_size = cmd-hdr.dxfer_len;
+ uint32_t sense;
+
+ DPRINTF(Emulate SCSI pthru cmd %x\n, cdb[0]);
+ sense = scsi_emulate_command(cmd-lun-bdrv, 0, cdb,
+cmd-iov[0].iov_len,
+cmd-iov[0].iov_base,
+cmd-sge_size);
+ sense_len = scsi_build_sense(cmd-sense, sense);
+ if (sense_len)
+ scsi_status = SAM_STAT_CHECK_CONDITION;
+ else
+ scsi_status = SAM_STAT_GOOD;
}
out:
megasas_unmap_sense(cmd, sense_len);
@@ -1105,13 +1120,16 @@ static int megasas_scsi_init(PCIDevice *dev)
lun-bdrv = NULL;
continue;
}
+ bdrv_set_tcq(lun-bdrv, 1);
/* check if we can use SG_IO */
ret = bdrv_ioctl(lun-bdrv, SG_IO, hdr);
if (ret) {
- DPRINTF(SCSI cmd passthrough not available on dev %d (error %d)\n,
+ DPRINTF(Using SCSI cmd emulation on dev %d (error %d)\n,
unit, ret);
- lun-sdev = NULL;
- lun-bdrv = NULL;
+ bdrv_set_sg(lun-bdrv, 0);
+ } else {
+ DPRINTF(Using SCSI cmd passthrough on dev %d\n, unit);
+ bdrv_set_sg(lun-bdrv, 1);
}
}
register_savevm(megasas, -1, 0, megasas_scsi_save, megasas_scsi_load, s);
--
1.6.0.2
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation
Am Wed 28 Oct 2009 02:58:33 PM CET schrieb Gerd Hoffmann kra...@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. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 0/4] megaraid_sas HBA emulation
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 h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Indefinite recursion in pci_default_read_config
Hi all,
I just triggered a nasty indefinite recursion in pci_default_read_config:
uint32_t pci_default_read_config(PCIDevice *d,
uint32_t address, int len)
{
uint32_t val = 0;
assert(len == 1 || len == 2 || len == 4);
if (pci_access_cap_config(d, address, len)) {
return d-cap.config_read(d, address, len);
}
len = MIN(len, pci_config_size(d) - address);
memcpy(val, d-config + address, len);
return le32_to_cpu(val);
}
And d-cap.config_read is pointing to pci_default_read_config:
(gdb) print *d
$3 = {qdev = {id = 0xc99b10 01:10.0, state = DEV_STATE_INITIALIZED,
opts = 0xc99ad0, hotplugged = 0, info = 0x837e60, parent_bus = 0xc71710,
num_gpio_out = 0, gpio_out = 0x0, num_gpio_in = 0, gpio_in = 0x0,
child_bus = {lh_first = 0x0}, num_child_bus = 0, sibling = {
le_next = 0xc99c30, le_prev = 0xc71730}},
config = 0xca3010 \206\200\312\020\003,
cmask = 0xca3120 \377\377\377\377, wmask = 0xca3230 ,
used = 0xca3340 , bus = 0xc71710, devfn = 32,
name = pci-assign, '\000' repeats 53 times, io_regions = {{
addr = 4060102656, size = 16384, filtered_size = 16384, type = 0 '\000',
map_func = 0x46a5f0 assigned_dev_iomem_map}, {addr = 0, size = 0,
filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 4060119040,
size = 16384, filtered_size = 16384, type = 0 '\000',
map_func = 0x46a5f0 assigned_dev_iomem_map}, {addr = 0, size = 0,
filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
filtered_size = 0, type = 0 '\000', map_func = 0}},
config_read = 0x46a050 assigned_dev_pci_read_config,
config_write = 0x469f30 assigned_dev_pci_write_config, irq = 0xca3450,
irq_state = 0 '\000', cap_present = 0, msix_cap = 0 '\000',
msix_entries_nr = 0, msix_table_page = 0x0, msix_mmio_index = 0,
msix_entry_used = 0x0, msix_bar_size = 0, version_id = 2,
msix_page_size = 0, msix_irq_entries = 0x0, cap = {supported = 1,
start = 64, length = 16,
config_read = 0x416770 pci_default_cap_read_config,
config_write = 0x46b750 assigned_device_pci_cap_write_config}}
Not good ...
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Indefinite recursion in pci_default_read_config
Michael S. Tsirkin wrote:
On Tue, Dec 15, 2009 at 12:59:41PM +0200, Avi Kivity wrote:
On 12/15/2009 12:57 PM, Hannes Reinecke wrote:
Hi all,
I just triggered a nasty indefinite recursion in pci_default_read_config:
uint32_t pci_default_read_config(PCIDevice *d,
uint32_t address, int len)
{
uint32_t val = 0;
assert(len == 1 || len == 2 || len == 4);
if (pci_access_cap_config(d, address, len)) {
return d-cap.config_read(d, address, len);
}
len = MIN(len, pci_config_size(d) - address);
memcpy(val, d-config + address, len);
return le32_to_cpu(val);
}
And d-cap.config_read is pointing to pci_default_read_config:
(gdb) print *d
$3 = {qdev = {id = 0xc99b10 01:10.0, state = DEV_STATE_INITIALIZED,
opts = 0xc99ad0, hotplugged = 0, info = 0x837e60, parent_bus =
0xc71710,
num_gpio_out = 0, gpio_out = 0x0, num_gpio_in = 0, gpio_in = 0x0,
child_bus = {lh_first = 0x0}, num_child_bus = 0, sibling = {
le_next = 0xc99c30, le_prev = 0xc71730}},
config = 0xca3010 \206\200\312\020\003,
cmask = 0xca3120 \377\377\377\377, wmask = 0xca3230 ,
used = 0xca3340 , bus = 0xc71710, devfn = 32,
name = pci-assign, '\000'repeats 53 times, io_regions = {{
addr = 4060102656, size = 16384, filtered_size = 16384, type = 0
'\000',
map_func = 0x46a5f0assigned_dev_iomem_map}, {addr = 0, size = 0,
filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size =
0,
filtered_size = 0, type = 0 '\000', map_func = 0}, {addr =
4060119040,
size = 16384, filtered_size = 16384, type = 0 '\000',
map_func = 0x46a5f0assigned_dev_iomem_map}, {addr = 0, size = 0,
filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size =
0,
filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size =
0,
filtered_size = 0, type = 0 '\000', map_func = 0}},
config_read = 0x46a050assigned_dev_pci_read_config,
config_write = 0x469f30assigned_dev_pci_write_config, irq = 0xca3450,
irq_state = 0 '\000', cap_present = 0, msix_cap = 0 '\000',
msix_entries_nr = 0, msix_table_page = 0x0, msix_mmio_index = 0,
msix_entry_used = 0x0, msix_bar_size = 0, version_id = 2,
msix_page_size = 0, msix_irq_entries = 0x0, cap = {supported = 1,
start = 64, length = 16,
config_read = 0x416770pci_default_cap_read_config,
config_write = 0x46b750assigned_device_pci_cap_write_config}}
Michael? This is likely a bad merge on my part. Can you help?
--
error compiling committee.c: too many arguments to function
Um, yes. I think the following is the right way to do this.
As a side note, we really should work to remove all these
hacks and make assignment use capability support
in upstream qemu.
--
diff --git a/hw/pci.c b/hw/pci.c
index 110a5fc..a74d3d4 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1016,19 +1016,26 @@ static void pci_update_irq_disabled(PCIDevice *d, int
was_irq_disabled)
}
}
+uint32_t pci_read_config(PCIDevice *d,
+ uint32_t address, int len)
+{
+uint32_t val = 0;
+
+len = MIN(len, pci_config_size(d) - address);
+memcpy(val, d-config + address, len);
+return le32_to_cpu(val);
+}
+
uint32_t pci_default_read_config(PCIDevice *d,
uint32_t address, int len)
{
-uint32_t val = 0;
assert(len == 1 || len == 2 || len == 4);
if (pci_access_cap_config(d, address, len)) {
return d-cap.config_read(d, address, len);
}
-len = MIN(len, pci_config_size(d) - address);
-memcpy(val, d-config + address, len);
-return le32_to_cpu(val);
+return pci_read_config(d, address, len);
}
static void pci_write_config(PCIDevice *pci_dev,
@@ -1052,7 +1059,7 @@ int pci_access_cap_config(PCIDevice *pci_dev, uint32_t
address, int len)
uint32_t pci_default_cap_read_config(PCIDevice *pci_dev,
uint32_t address, int len)
{
-return pci_default_read_config(pci_dev, address, len);
+return pci_read_config(pci_dev, address, len);
}
void pci_default_cap_write_config(PCIDevice *pci_dev,
Ok, works. Except for a missing prototype in hw/pci.h :-)
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] [PATCH] SCSI passthrough for virtio-blk
Hi all,
I got bored and implemented SCSI passthrough for the virtio-blk driver.
Principle is quite simple, just put the missing fields (cdb, sense and
status header) on the virtio queue and then call the SG_IO ioctl on the
host.
So when using '-drive file=/dev/sgXX,if=virtio,format=host_device' you
can happily call any sg_XX command on the resulting vdX device. Quite
neat, methinks. And it's even backwards compatible, so each of these
patches should work without the other one applied.
As one would have guessed there are two patches, one for the linux
kernel to modify the virtio-blk driver in the guest and one for the
qemu/kvm userland program to modify the virtio-blk driver on the host.
This patch is relative to avi's kvm-userland tree from kernel.org.
As usual, comments etc to me.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
[EMAIL PROTECTED] +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
virtio: Implement SCSI passthrough for virtio-blk
This patch implements SCSI passthrough for any virtio-blk device.
The data on the virtio queue will only be modified for a SCSI command,
so the normal I/O flow is unchanged.
Signed-off-by: Hannes Reinecke [EMAIL PROTECTED]
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 4225109..46f03d2 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -35,6 +35,7 @@ struct virtblk_req
struct list_head list;
struct request *req;
struct virtio_blk_outhdr out_hdr;
+ struct virtio_blk_inhdr in_hdr;
u8 status;
};
@@ -47,20 +48,29 @@ static void blk_done(struct virtqueue *vq)
spin_lock_irqsave(vblk-lock, flags);
while ((vbr = vblk-vq-vq_ops-get_buf(vblk-vq, len)) != NULL) {
- int uptodate;
+ int error;
+ unsigned int bytes;
switch (vbr-status) {
case VIRTIO_BLK_S_OK:
- uptodate = 1;
+ error = 0;
break;
case VIRTIO_BLK_S_UNSUPP:
- uptodate = -ENOTTY;
+ error = -ENOTTY;
break;
default:
- uptodate = 0;
+ error = -EIO;
break;
}
- end_dequeued_request(vbr-req, uptodate);
+ if (blk_pc_request(vbr-req)) {
+ vbr-req-data_len = vbr-in_hdr.residual;
+ bytes = vbr-in_hdr.data_len;
+ vbr-req-sense_len = vbr-in_hdr.sense_len;
+ vbr-req-errors = vbr-in_hdr.status;
+ } else
+ bytes = blk_rq_bytes(vbr-req);
+
+ __blk_end_request(vbr-req, error, bytes);
list_del(vbr-list);
mempool_free(vbr, vblk-pool);
}
@@ -72,7 +82,7 @@ static void blk_done(struct virtqueue *vq)
static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
struct request *req)
{
- unsigned long num, out, in;
+ unsigned long num, out = 0, in = 0;
struct virtblk_req *vbr;
vbr = mempool_alloc(vblk-pool, GFP_ATOMIC);
@@ -99,20 +109,31 @@ static bool do_req(struct request_queue *q, struct
virtio_blk *vblk,
/* This init could be done at vblk creation time */
sg_init_table(vblk-sg, VIRTIO_MAX_SG);
- sg_set_buf(vblk-sg[0], vbr-out_hdr, sizeof(vbr-out_hdr));
- num = blk_rq_map_sg(q, vbr-req, vblk-sg+1);
- sg_set_buf(vblk-sg[num+1], vbr-status, sizeof(vbr-status));
-
- if (rq_data_dir(vbr-req) == WRITE) {
- vbr-out_hdr.type |= VIRTIO_BLK_T_OUT;
- out = 1 + num;
- in = 1;
- } else {
- vbr-out_hdr.type |= VIRTIO_BLK_T_IN;
- out = 1;
- in = 1 + num;
+ sg_set_buf(vblk-sg[out], vbr-out_hdr, sizeof(vbr-out_hdr));
+ out++;
+ if (blk_pc_request(vbr-req)) {
+ sg_set_buf(vblk-sg[out], vbr-req-cmd, vbr-req-cmd_len);
+ out++;
+ }
+ num = blk_rq_map_sg(q, vbr-req, vblk-sg+out);
+ if (blk_pc_request(vbr-req)) {
+ sg_set_buf(vblk-sg[num+out+in], vbr-req-sense, 96);
+ in++;
+ sg_set_buf(vblk-sg[num+out+in], vbr-in_hdr,
+ sizeof(vbr-in_hdr));
+ in++;
+ }
+ sg_set_buf(vblk-sg[num+out+in], vbr-status, sizeof(vbr-status));
+ in++;
+ if (num) {
+ if (rq_data_dir(vbr-req) == WRITE) {
+ vbr-out_hdr.type |= VIRTIO_BLK_T_OUT;
+ out += num;
+ } else {
+ vbr-out_hdr.type |= VIRTIO_BLK_T_IN;
+ in += num
Re: [RFC] [PATCH] SCSI passthrough for virtio-blk
Hi Christian, Christian Borntraeger wrote: Am Freitag, 29. August 2008 schrieb Hannes Reinecke: So when using '-drive file=/dev/sgXX,if=virtio,format=host_device' you can happily call any sg_XX command on the resulting vdX device. Quite neat, methinks. And it's even backwards compatible, so each of these patches should work without the other one applied. Does not work here. If the host does not support the pass-through, the device drivers waits for an response. I tried sdparm /dev/vda with a patched kernel and an unpatched userspace. Hmm. Works here, using an unpatched kvm-73. Which version did you use? Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage [EMAIL PROTECTED] +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg) -- 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: [RFC] [PATCH] SCSI passthrough for virtio-blk
Hi Christian, Christian Borntraeger wrote: Am Freitag, 29. August 2008 schrieb Hannes Reinecke: Hmm. Works here, using an unpatched kvm-73. Which version did you use? I use the s390 userspace prototype kuli which uses an virtio transport similar to lguest. I retried and it seems to race. Most of the time it works fine, but sometimes sdparm hangs. I will have a 2nd look. sysrq-t gives me the following trace: Call Trace: ([04000755bc78] 0x4000755bc78) sdparmD 0043659e 0 2493 1 0012004a 0744f740 0744f778 001896469fd23785 0744f778 009e5500 0043f230 00120130 0744f778 06d39400 06d39f80 0001 009e6f00 076bf8e8 0744f7c8 07530670 0043f610 00435e66 0744f7c8 0744f868 Call Trace: ([00435e66] schedule+0x32e/0x7ec) [0043659e] schedule_timeout+0xba/0x10c [004358da] wait_for_common+0xbe/0x1a8 [0027ec3e] blk_execute_rq+0x86/0xc4 [00282768] sg_io+0x1a4/0x360 [00282f8c] scsi_cmd_ioctl+0x2bc/0x3f0 [002c3108] virtblk_ioctl+0x44/0x58 [0027ff18] blkdev_driver_ioctl+0x98/0xa4 [0027ffd8] blkdev_ioctl+0xb4/0x7f8 [001e1572] block_ioctl+0x3a/0x48 [001bca0a] vfs_ioctl+0x52/0xdc [001bcb0a] do_vfs_ioctl+0x76/0x350 [001bce6e] sys_ioctl+0x8a/0xa0 [0011282c] sysc_tracego+0xe/0x14 [02114286] 0x2114286 I'm tempted to say 'not my fault'; the submitted SCSI request on the _host_ hangs and doesn't come back. Looks more like a SCSI problem on the host ... Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage [EMAIL PROTECTED] +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg) -- 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] lsi53c895a: Add support for OS/2 Warp SYM8XX.ADD driver
Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Greetings Paul, Jan, Kevin and co, This series is against my v0.12.5 qemu-kvm.git that contains QEMU SCSI layer SGL passthrough from Gerd Hoffman, 8708EM2 MegaSas emulation from Dr. Hannes Reinecke, and well as my own hw/scsi-bsg.c support. This tree is located here: http://git.kernel.org/?p=virt/kvm/nab/qemu-kvm.git;a=summary This first patch adds a missing qdev-reset() NOP caller in hw/scsi-generic.c that is now expected by lsi53c895a.c in = v0.12.5 code. You will want to apply this to all = v0.12.5 QEMU trees so scsi-generic does not segfault with lsi53c895a expecting a valid qdev-reset(). The second item is a bit more exotic.. 8-) So I have been thinking about how to get qemu-kvm.git scsi-generic - TCM_Loop to function with OS/2 Warp v4 (SP15) in guest for a while now, and I am happy to report that after sending some time in the last weeks getting OS/2 setup (hey, it has been +13 years) and finding a functioning sym53c895a driver, and finally finding a working SYM8XX.ADD and being able to fill in missing informational registers and adding a workaround to fix a bogus Destination ID register WRITE from the now +10 year old SYM8XX.ADD driver code. Hey, and while you're at it: The lsi53c895a emulation is missing support for 'abort' and 'device reset' TMFs; newer Linux kernel have the habit of sending them accordingly. Should be fairly straightforward, only I gave up on it after reading the first 10 odd pages of the manual ... Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
PCI device not properly reset after VFIO
Hi Alex, I've been playing around with VFIO and megasas (of course). What I did now was switching between VFIO and 'normal' operation, ie emulated access. megasas is happily running under VFIO, but when I do an emergency stop like killing the Qemu session the PCI device is not properly reset. IE when I load 'megaraid_sas' after unbinding the vfio_pci module the driver cannot initialize the card and waits forever for the firmware state to change. I need to do a proper pci reset via echo 1 /sys/bus/pci/device//reset to get it into a working state again. Looking at vfio_pci_disable() pci reset is called before the config state and BARs are restored. Seeing that vfio_pci_enable() calls pci reset right at the start, too, before modifying anything I do wonder whether the pci reset is at the correct location for disable. I would have expected to call pci reset in vfio_pci_disable() _after_ we have restored the configuration, to ensure a sane state after reset. And, as experience show, we do need to call it there. So what is the rationale for the pci reset? Can we move it to the end of vfio_pci_disable() or do we need to call pci reset twice? Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PCI device not properly reset after VFIO
On 10/18/2012 04:40 PM, Alex Williamson wrote:
Hi Hannes,
Thanks for testing vfio
On Thu, 2012-10-18 at 08:47 +0200, Hannes Reinecke wrote:
Hi Alex,
I've been playing around with VFIO and megasas (of course).
What I did now was switching between VFIO and 'normal' operation, ie
emulated access.
megasas is happily running under VFIO, but when I do an emergency
stop like killing the Qemu session the PCI device is not properly reset.
IE when I load 'megaraid_sas' after unbinding the vfio_pci module
the driver cannot initialize the card and waits forever for the
firmware state to change.
I need to do a proper pci reset via
echo 1 /sys/bus/pci/device//reset
to get it into a working state again.
Looking at vfio_pci_disable() pci reset is called before the config
state and BARs are restored.
Seeing that vfio_pci_enable() calls pci reset right at the start,
too, before modifying anything I do wonder whether the pci reset is
at the correct location for disable.
I would have expected to call pci reset in vfio_pci_disable()
_after_ we have restored the configuration, to ensure a sane state
after reset.
And, as experience show, we do need to call it there.
So what is the rationale for the pci reset?
Can we move it to the end of vfio_pci_disable() or do we need to
call pci reset twice?
I believe the rationale was that by resetting the device before we
restore the state we stop anything that the device was doing. Restoring
the saved state on a running device seems like it could cause problems,
so you may be right and we actually need to do reset, load, restore,
reset. Does adding another call to pci_reset_function in the
pci_restore_state (as below) solve the problem? Traditional KVM device
assignment has a nearly identical path, does it have this same bug?
It's actually the first time I've been able to test this (the
hardware is a bit tricky to setup ...), so I cannot tell (yet)
if KVM exhibited the same thing.
Thanks,
Alex
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 6c11994..d07a45c 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -107,9 +107,10 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev)
pci_reset_function(vdev-pdev);
if (pci_load_and_free_saved_state(vdev-pdev,
- vdev-pci_saved_state) == 0)
+ vdev-pci_saved_state) == 0) {
pci_restore_state(vdev-pdev);
- else
+ pci_reset_function(vdev-pdev);
+ } else
pr_info(%s: Couldn't reload %s saved state\n,
__func__, dev_name(vdev-pdev-dev));
I would have called reset after unmapping the BARs; the HBA I'm
working with does need to access the BARs, so the content of them
might be relevant, too.
But then I'm not really a PCI expert.
Maybe we should ask Tony Luck or Bjorn Helgaas.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio scsi host draft specification, v3
On 07/01/2011 08:41 AM, Paolo Bonzini wrote: On 06/29/2011 11:39 AM, Stefan Hajnoczi wrote: Of course, when doing so we would be lose the ability to freely remap LUNs. But then remapping LUNs doesn't gain you much imho. Plus you could always use qemu block backend here if you want to hide the details. And you could always use the QEMU block backend with scsi-generic if you want to remap LUNs, instead of true passthrough via the kernel target. IIUC the in-kernel target always does remapping. It passes through individual LUNs rather than entire targets and you pick LU Numbers to map to the backing storage (which may or may not be a SCSI pass-through device). Nicholas Bellinger can confirm whether this is correct. But then I don't understand. If you pick LU numbers both with the in-kernel target and with QEMU, you do not need to use e.g. WWPNs with fiber channel, because we are not passing through the details of the transport protocol (one day we might have virtio-fc, but more likely not). So the LUNs you use might as well be represented by hierarchical LUNs. Actually, the kernel does _not_ do a LUN remapping. It just so happens that most storage arrays will present the LUN starting with 0, so normally you wouldn't notice. However, some arrays have an array-wide LUN range, so you start seeing LUNs at odd places: [3:0:5:0]diskLSI INF-01-000750 /dev/sdw [3:0:5:7]diskLSI Universal Xport 0750 /dev/sdx Using NPIV with KVM would be done by mapping the same virtual N_Port ID in the host(s) to the same LU number in the guest. You might already do this now with virtio-blk, in fact. The point here is not the mapping. The point is rescanning. You can map existing NPIV devices already. But you _cannot_ rescan the host/device whatever _from the guest_ to detect if new devices are present. That is the problem I'm trying to describe here. To be more explicit: Currently you have to map existing devices directly as individual block or scsi devices to the guest. And rescan within the guest can only be sent to that device, so the only information you will get able to gather is if the device itself is still present. You are unable to detect if there are other devices attached to your guest which you should connect to. So we have to have an enclosing instance (ie the equivalent of a SCSI target), which is capable of telling us exactly this. Put in another way: the virtio-scsi device is itself a SCSI target, so yes, there is a single target port identifier in virtio-scsi. But this SCSI target just passes requests down to multiple real targets, and so will let you do ALUA and all that. Argl. No way. The virtio-scsi device has to map to a single LUN. I thought I mentioned this already, but I'd better clarify this again: The SCSI spec itself only deals with LUNs, so anything you'll read in there obviously will only handle the interaction between the initiator (read: host) and the LUN itself. However, the actual command is send via an intermediat target, hence you'll always see the reference to the ITL (initiator-target-lun) nexus. The SCSI spec details discovery of the individual LUNs presented by a given target, it does _NOT_ detail the discovery of the targets themselves. That is being delegated to the underlying transport, in most cases SAS or FibreChannel. For the same reason the SCSI spec can afford to disdain any reference to path failure, device hot-plugging etc; all of these things are being delegated to the transport. In our context the virtio-scsi device should map to the LUN, and the virtio-scsi _host_ backend should map to the target. The virtio-scsi _guest_ driver will then map to the initiator. So we should be able to attach more than one device to the backend, which then will be presented to the initiator. In the case of NPIV it would make sense to map the virtual SCSI host to the backend, so that all devices presented to the virtual SCSI host will be presented to the backend, too. However, when doing so these devices will normally be referenced by their original LUN, as these will be presented to the guest via eg 'REPORT LUNS'. The above thread now tries to figure out if we should remap those LUN numbers or just expose them as they are. If we decide on remapping, we have to emulate _all_ commands referring explicitely to those LUN numbers (persistent reservations, anyone?). If we don't, we would expose some hardware detail to the guest, but would save us _a lot_ of processing. I'm all for the latter. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http
[PATCH 0/3] [v4] Megasas HBA emulation
Hi all, thanks to Paolo and Stefan most of the SCSI patches are now in, so I've made the next attempt of submitting my Megaraid SAS HBA emulation. To do so, I've done two additional patches, both should be valid cleanups. - Replace 'tag' by 'hba_private' The SCSIRequest structure has a 'tag', which is being used by the drivers to match the SCSIRequest to the internal request structure. The only driver actually to benefit from this is the lsi53c895a driver, everyone else either leaves it blank or uses some internal numberting here. So this patch converts the 'tag' to a 'hba_private' pointer, which allows the driver to store a pointer to the internal structure directly within the SCSIRequest. This saves the lookup and an additional field in the driver internal request structure. - Add an 'offset' parameter to iov_to_buf() iov_from_buf() has it, but iov_to_buf() has it not. But we'll be needing it if the iovec is larger than the buffer. So there. And, of course, the megasas driver itself. Which has been modified to work with the new interface; otherwise there have been no changes to the previous submission. Hannes Reinecke (3): iov: Add 'offset' parameter to iov_to_buf() scsi: replace 'tag' with 'hba_private' pointer megasas: LSI Megaraid SAS emulation Makefile.objs |1 + default-configs/pci.mak |1 + hw/esp.c|2 +- hw/lsi53c895a.c | 17 +- hw/megasas.c| 1923 +++ hw/mfi.h| 1197 + hw/pci_ids.h|3 +- hw/scsi-bus.c | 22 +- hw/scsi-disk.c |5 +- hw/scsi-generic.c |4 +- hw/scsi.h |8 +- hw/spapr_vscsi.c| 41 +- hw/usb-msd.c| 10 +- hw/virtio-net.c |2 +- hw/virtio-serial-bus.c |2 +- iov.c | 23 +- iov.h |2 +- trace-events| 14 +- 18 files changed, 3193 insertions(+), 84 deletions(-) create mode 100644 hw/megasas.c create mode 100644 hw/mfi.h -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] iov: Add 'offset' parameter to iov_to_buf()
Occasionally, the buffer needs to be placed at a offset within
the iovec when copying the buffer to the iovec.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/virtio-net.c|2 +-
hw/virtio-serial-bus.c |2 +-
iov.c | 23 ++-
iov.h |2 +-
4 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 6997e02..a32cc01 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -657,7 +657,7 @@ static ssize_t virtio_net_receive(VLANClientState *nc,
const uint8_t *buf, size_
/* copy in packet. ugh */
len = iov_from_buf(sg, elem.in_num,
- buf + offset, size - offset);
+ buf + offset, 0, size - offset);
total += len;
offset += len;
/* If buffers can't be merged, at this point we
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 7f6db7b..53c58d0 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -103,7 +103,7 @@ static size_t write_to_port(VirtIOSerialPort *port,
}
len = iov_from_buf(elem.in_sg, elem.in_num,
- buf + offset, size - offset);
+ buf + offset, 0, size - offset);
offset += len;
virtqueue_push(vq, elem, len);
diff --git a/iov.c b/iov.c
index 588cd04..9ead6ee 100644
--- a/iov.c
+++ b/iov.c
@@ -15,21 +15,26 @@
#include iov.h
size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
-const void *buf, size_t size)
+const void *buf, size_t offset, size_t size)
{
-size_t offset;
+size_t iov_off, buf_off;
unsigned int i;
-offset = 0;
-for (i = 0; offset size i iovcnt; i++) {
-size_t len;
+iov_off = 0;
+buf_off = 0;
+for (i = 0; i iovcnt size; i++) {
+if (offset (iov_off + iov[i].iov_len)) {
+size_t len = MIN((iov_off + iov[i].iov_len) - offset, size);
-len = MIN(iov[i].iov_len, size - offset);
+memcpy(iov[i].iov_base + (offset - iov_off), buf + buf_off, len);
-memcpy(iov[i].iov_base, buf + offset, len);
-offset += len;
+buf_off += len;
+offset += len;
+size -= len;
+}
+iov_off += iov[i].iov_len;
}
-return offset;
+return buf_off;
}
size_t iov_to_buf(const struct iovec *iov, const unsigned int iovcnt,
diff --git a/iov.h b/iov.h
index 60a8547..2677527 100644
--- a/iov.h
+++ b/iov.h
@@ -13,7 +13,7 @@
#include qemu-common.h
size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
-const void *buf, size_t size);
+const void *buf, size_t offset, size_t size);
size_t iov_to_buf(const struct iovec *iov, const unsigned int iovcnt,
void *buf, size_t offset, size_t size);
size_t iov_size(const struct iovec *iov, const unsigned int iovcnt);
--
1.6.0.2
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] scsi: replace 'tag' with 'hba_private' pointer
'tag' is just an abstraction to identify the command
from the driver. So we should make that explicit by
replacing 'tag' with a driver-defined pointer 'hba_private'.
This saves the lookup for driver handling several commands
in parallel.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/esp.c |2 +-
hw/lsi53c895a.c | 17 -
hw/scsi-bus.c | 22 +++---
hw/scsi-disk.c|5 ++---
hw/scsi-generic.c |4 ++--
hw/scsi.h |8
hw/spapr_vscsi.c | 41 -
hw/usb-msd.c | 10 +-
trace-events | 14 +++---
9 files changed, 52 insertions(+), 71 deletions(-)
diff --git a/hw/esp.c b/hw/esp.c
index 6d3f5d2..912ff89 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -244,7 +244,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t
busid)
DPRINTF(do_busid_cmd: busid 0x%x\n, busid);
lun = busid 7;
-s-current_req = scsi_req_new(s-current_dev, 0, lun);
+s-current_req = scsi_req_new(s-current_dev, lun, s);
datalen = scsi_req_enqueue(s-current_req, buf);
s-ti_size = datalen;
if (datalen != 0) {
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 940b43a..272e919 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -670,7 +670,7 @@ static void lsi_request_cancelled(SCSIRequest *req)
return;
}
-p = lsi_find_by_tag(s, req-tag);
+p = req-hba_private;
if (p) {
QTAILQ_REMOVE(s-queue, p, next);
scsi_req_unref(req);
@@ -680,18 +680,17 @@ static void lsi_request_cancelled(SCSIRequest *req)
/* Record that data is available for a queued command. Returns zero if
the device was reselected, nonzero if the IO is deferred. */
-static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t len)
+static int lsi_queue_req(LSIState *s, SCSIRequest *req, uint32_t len)
{
-lsi_request *p;
+lsi_request *p = req-hba_private;
-p = lsi_find_by_tag(s, tag);
if (!p) {
-BADF(IO with unknown tag %d\n, tag);
+BADF(IO with unknown reference %p\n, req-hba_private);
return 1;
}
if (p-pending) {
-BADF(Multiple IO pending for tag %d\n, tag);
+BADF(Multiple IO pending for request %p\n, p);
}
p-pending = len;
/* Reselect if waiting for it, or if reselection triggers an IRQ
@@ -743,9 +742,9 @@ static void lsi_transfer_data(SCSIRequest *req, uint32_t
len)
LSIState *s = DO_UPCAST(LSIState, dev.qdev, req-bus-qbus.parent);
int out;
-if (s-waiting == 1 || !s-current || req-tag != s-current-tag ||
+if (s-waiting == 1 || !s-current || req-hba_private != s-current ||
(lsi_irq_on_rsl(s) !(s-scntl1 LSI_SCNTL1_CON))) {
-if (lsi_queue_tag(s, req-tag, len)) {
+if (lsi_queue_req(s, req, len)) {
return;
}
}
@@ -789,7 +788,7 @@ static void lsi_do_command(LSIState *s)
assert(s-current == NULL);
s-current = qemu_mallocz(sizeof(lsi_request));
s-current-tag = s-select_tag;
-s-current-req = scsi_req_new(dev, s-current-tag, s-current_lun);
+s-current-req = scsi_req_new(dev, s-current_lun, s-current);
n = scsi_req_enqueue(s-current-req, buf);
if (n) {
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index ad6a730..d1fc481 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -131,7 +131,7 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
return res;
}
-SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t
lun)
+SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t lun, void
*hba_private)
{
SCSIRequest *req;
@@ -139,16 +139,16 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d,
uint32_t tag, uint32_t l
req-refcount = 1;
req-bus = scsi_bus_from_device(d);
req-dev = d;
-req-tag = tag;
req-lun = lun;
+req-hba_private = hba_private;
req-status = -1;
-trace_scsi_req_alloc(req-dev-id, req-lun, req-tag);
+trace_scsi_req_alloc(req-dev-id, req-lun, req-hba_private);
return req;
}
-SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun)
+SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t lun, void *hba_private)
{
-return d-info-alloc_req(d, tag, lun);
+return d-info-alloc_req(d, lun, hba_private);
}
uint8_t *scsi_req_get_buf(SCSIRequest *req)
@@ -182,7 +182,7 @@ int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
static void scsi_req_dequeue(SCSIRequest *req)
{
-trace_scsi_req_dequeue(req-dev-id, req-lun, req-tag);
+trace_scsi_req_dequeue(req-dev-id, req-lun, req-hba_private);
if (req-enqueued) {
QTAILQ_REMOVE(req-dev-requests, req, next);
req-enqueued = false;
@@ -214,7 +214,7 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
req-cmd.len = 12;
break;
default:
-trace_scsi_req_parse_bad(req-dev-id, req-lun, req-tag, cmd[0
Re: [PATCH 1/3] iov: Add 'offset' parameter to iov_to_buf()
On 07/01/2011 10:03 AM, Paolo Bonzini wrote: On 07/01/2011 09:42 AM, Hannes Reinecke wrote: size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt, - const void *buf, size_t size) + const void *buf, size_t offset, size_t size) Wrong commit subject, it seems. :) Bummer. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] iov: Add 'offset' parameter to iov_to_buf()
On 07/01/2011 10:02 AM, Alexander Graf wrote: On 01.07.2011, at 09:42, Hannes Reinecke wrote: Occasionally, the buffer needs to be placed at a offset within the iovec when copying the buffer to the iovec. So this is a buffer into the iovec, right? Wouldn't it make sense to also modify iov_to_buf respectively then, so the API stays similar? Ahem. That's exactly what the patch does. Except from the mixed-up subject. iov_to_buff() has an offset parameter, iov_from_buf() has not. For no obvious reasons. Also, it'd be nice to give the parameter a more obvious name, so potential users can easily recognize what it offsets. Yes, that sounds reasonable. What about 'iov_off' ? (And possibly rename 'iovcnt' to 'iov_cnt' for consistency ?) Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] scsi: replace 'tag' with 'hba_private' pointer
On 07/01/2011 10:27 AM, Paolo Bonzini wrote:
On 07/01/2011 09:42 AM, Hannes Reinecke wrote:
'tag' is just an abstraction to identify the command
from the driver. So we should make that explicit by
replacing 'tag' with a driver-defined pointer 'hba_private'.
This saves the lookup for driver handling several commands
in parallel.
This makes tracing a bit harder to follow. Perhaps you can keep the
transport tag (a uint64_t) in the SCSIRequest for debugging purposes?
Sure. Anything to get the patches accepted :-)
Signed-off-by: Hannes Reineckeh...@suse.de
---
hw/esp.c | 2 +-
hw/lsi53c895a.c | 17 -
hw/scsi-bus.c | 22 +++---
hw/scsi-disk.c | 5 ++---
hw/scsi-generic.c | 4 ++--
hw/scsi.h | 8
hw/spapr_vscsi.c | 41 -
hw/usb-msd.c | 10 +-
trace-events | 14 +++---
9 files changed, 52 insertions(+), 71 deletions(-)
diff --git a/hw/esp.c b/hw/esp.c
index 6d3f5d2..912ff89 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -244,7 +244,7 @@ static void do_busid_cmd(ESPState *s, uint8_t
*buf, uint8_t busid)
DPRINTF(do_busid_cmd: busid 0x%x\n, busid);
lun = busid 7;
- s-current_req = scsi_req_new(s-current_dev, 0, lun);
+ s-current_req = scsi_req_new(s-current_dev, lun, s);
Might as well pass NULL here. The hba_private value is basically
unnecessary when the adapter doesn't support tagged command queuing.
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 86582cc..4e2ea03 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -216,8 +216,8 @@ static void usb_msd_transfer_data(SCSIRequest
*req, uint32_t len)
MSDState *s = DO_UPCAST(MSDState, dev.qdev, req-bus-qbus.parent);
USBPacket *p = s-packet;
- if (req-tag != s-tag) {
- fprintf(stderr, usb-msd: Unexpected SCSI Tag 0x%x\n, req-tag);
+ if (req-hba_private != s) {
+ fprintf(stderr, usb-msd: Unexpected SCSI command 0x%p\n, req);
}
Same here, just pass NULL and remove these ifs.
Otherwise looks like a very good idea.
Ok, I'll be resending both.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] scsi: replace 'tag' with 'hba_private' pointer
On 07/01/2011 10:27 AM, Paolo Bonzini wrote: On 07/01/2011 09:42 AM, Hannes Reinecke wrote: 'tag' is just an abstraction to identify the command from the driver. So we should make that explicit by replacing 'tag' with a driver-defined pointer 'hba_private'. This saves the lookup for driver handling several commands in parallel. This makes tracing a bit harder to follow. Perhaps you can keep the transport tag (a uint64_t) in the SCSIRequest for debugging purposes? Hmm. The transport tag wouldn't have any meaning outside scsi-bus.c. And it's a 64-bit value. So why can't we use the hba_private pointer directly here? After some I/O has been ongoing the linear 'tag' number becomes unreadable very quickly, so there's not much difference here ... Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] [v5] Megasas HBA Emulation
Hi all, after getting various feedback from Paolo, Stefan, and Alexander I've respun the patches. Chances since the previous version: - iov: Update parameter usage in iov_(to|from)_buf() Updated description for the first patch and clarified the usage Renamed arguments for io_XXX for clarification - scsi: Add 'hba_private' to SCSIRequest Kept 'tag' for tracing and just add 'hba_private' as an additional field as per request from Paolo - megasas: checkpatch.pl fixes and update to work with the changed interface in scsi_req_new(). Also included the suggested fixes from Alex. Hannes Reinecke (3): iov: Update parameter usage in iov_(to|from)_buf() scsi: Add 'hba_private' to SCSIRequest megasas: LSI Megaraid SAS emulation Makefile.objs |1 + default-configs/pci.mak |1 + hw/esp.c|2 +- hw/lsi53c895a.c | 22 +- hw/megasas.c| 1920 +++ hw/mfi.h| 1197 + hw/pci_ids.h|3 +- hw/scsi-bus.c |9 +- hw/scsi-disk.c |4 +- hw/scsi-generic.c |5 +- hw/scsi.h | 10 +- hw/spapr_vscsi.c| 29 +- hw/usb-msd.c|9 +- hw/virtio-net.c |2 +- hw/virtio-serial-bus.c |2 +- iov.c | 49 +- iov.h | 10 +- 17 files changed, 3192 insertions(+), 83 deletions(-) create mode 100644 hw/megasas.c create mode 100644 hw/mfi.h -- 1.7.3.4 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] iov: Update parameter usage in iov_(to|from)_buf()
iov_to_buf() has an 'offset' parameter, iov_from_buf() hasn't.
This patch adds the missing parameter to iov_from_buf().
It also renames the 'offset' parameter to 'iov_off' to
emphasize it's the offset into the iovec and not the buffer.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/virtio-net.c|2 +-
hw/virtio-serial-bus.c |2 +-
iov.c | 49 ++-
iov.h | 10
4 files changed, 34 insertions(+), 29 deletions(-)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 6997e02..a32cc01 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -657,7 +657,7 @@ static ssize_t virtio_net_receive(VLANClientState *nc,
const uint8_t *buf, size_
/* copy in packet. ugh */
len = iov_from_buf(sg, elem.in_num,
- buf + offset, size - offset);
+ buf + offset, 0, size - offset);
total += len;
offset += len;
/* If buffers can't be merged, at this point we
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 7f6db7b..53c58d0 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -103,7 +103,7 @@ static size_t write_to_port(VirtIOSerialPort *port,
}
len = iov_from_buf(elem.in_sg, elem.in_num,
- buf + offset, size - offset);
+ buf + offset, 0, size - offset);
offset += len;
virtqueue_push(vq, elem, len);
diff --git a/iov.c b/iov.c
index 588cd04..1e02791 100644
--- a/iov.c
+++ b/iov.c
@@ -14,56 +14,61 @@
#include iov.h
-size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
-const void *buf, size_t size)
+size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt,
+const void *buf, size_t iov_off, size_t size)
{
-size_t offset;
+size_t iovec_off, buf_off;
unsigned int i;
-offset = 0;
-for (i = 0; offset size i iovcnt; i++) {
-size_t len;
+iovec_off = 0;
+buf_off = 0;
+for (i = 0; i iov_cnt size; i++) {
+if (iov_off (iovec_off + iov[i].iov_len)) {
+size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off, size);
-len = MIN(iov[i].iov_len, size - offset);
+memcpy(iov[i].iov_base + (iov_off - iovec_off), buf + buf_off,
len);
-memcpy(iov[i].iov_base, buf + offset, len);
-offset += len;
+buf_off += len;
+iov_off += len;
+size -= len;
+}
+iovec_off += iov[i].iov_len;
}
-return offset;
+return buf_off;
}
-size_t iov_to_buf(const struct iovec *iov, const unsigned int iovcnt,
- void *buf, size_t offset, size_t size)
+size_t iov_to_buf(const struct iovec *iov, const unsigned int iov_cnt,
+ void *buf, size_t iov_off, size_t size)
{
uint8_t *ptr;
-size_t iov_off, buf_off;
+size_t iovec_off, buf_off;
unsigned int i;
ptr = buf;
-iov_off = 0;
+iovec_off = 0;
buf_off = 0;
-for (i = 0; i iovcnt size; i++) {
-if (offset (iov_off + iov[i].iov_len)) {
-size_t len = MIN((iov_off + iov[i].iov_len) - offset , size);
+for (i = 0; i iov_cnt size; i++) {
+if (iov_off (iovec_off + iov[i].iov_len)) {
+size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off , size);
-memcpy(ptr + buf_off, iov[i].iov_base + (offset - iov_off), len);
+memcpy(ptr + buf_off, iov[i].iov_base + (iov_off - iovec_off),
len);
buf_off += len;
-offset += len;
+iov_off += len;
size -= len;
}
-iov_off += iov[i].iov_len;
+iovec_off += iov[i].iov_len;
}
return buf_off;
}
-size_t iov_size(const struct iovec *iov, const unsigned int iovcnt)
+size_t iov_size(const struct iovec *iov, const unsigned int iov_cnt)
{
size_t len;
unsigned int i;
len = 0;
-for (i = 0; i iovcnt; i++) {
+for (i = 0; i iov_cnt; i++) {
len += iov[i].iov_len;
}
return len;
diff --git a/iov.h b/iov.h
index 60a8547..110f67a 100644
--- a/iov.h
+++ b/iov.h
@@ -12,8 +12,8 @@
#include qemu-common.h
-size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
-const void *buf, size_t size);
-size_t iov_to_buf(const struct iovec *iov, const unsigned int iovcnt,
- void *buf, size_t offset, size_t size);
-size_t iov_size(const struct iovec *iov, const unsigned int iovcnt);
+size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt,
+const void *buf, size_t iov_off, size_t size);
+size_t iov_to_buf(const struct iovec *iov, const unsigned int iov_cnt,
+ void *buf, size_t iov_off, size_t size);
+size_t iov_size(const struct iovec *iov, const unsigned int iov_cnt);
--
1.7.3.4
--
To unsubscribe from this list: send the line
[PATCH 2/3] scsi: Add 'hba_private' to SCSIRequest
'tag' is just an abstraction to identify the command
from the driver. So we should make that explicit by
replacing 'tag' with a driver-defined pointer 'hba_private'.
This saves the lookup for driver handling several commands
in parallel.
'tag' is still being kept for tracing purposes.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/esp.c |2 +-
hw/lsi53c895a.c | 22 --
hw/scsi-bus.c |9 ++---
hw/scsi-disk.c|4 ++--
hw/scsi-generic.c |5 +++--
hw/scsi.h | 10 +++---
hw/spapr_vscsi.c | 29 +
hw/usb-msd.c |9 +
8 files changed, 37 insertions(+), 53 deletions(-)
diff --git a/hw/esp.c b/hw/esp.c
index 6d3f5d2..aa87197 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -244,7 +244,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t
busid)
DPRINTF(do_busid_cmd: busid 0x%x\n, busid);
lun = busid 7;
-s-current_req = scsi_req_new(s-current_dev, 0, lun);
+s-current_req = scsi_req_new(s-current_dev, 0, lun, NULL);
datalen = scsi_req_enqueue(s-current_req, buf);
s-ti_size = datalen;
if (datalen != 0) {
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 940b43a..69eec1d 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -661,7 +661,7 @@ static lsi_request *lsi_find_by_tag(LSIState *s, uint32_t
tag)
static void lsi_request_cancelled(SCSIRequest *req)
{
LSIState *s = DO_UPCAST(LSIState, dev.qdev, req-bus-qbus.parent);
-lsi_request *p;
+lsi_request *p = req-hba_private;
if (s-current req == s-current-req) {
scsi_req_unref(req);
@@ -670,7 +670,6 @@ static void lsi_request_cancelled(SCSIRequest *req)
return;
}
-p = lsi_find_by_tag(s, req-tag);
if (p) {
QTAILQ_REMOVE(s-queue, p, next);
scsi_req_unref(req);
@@ -680,18 +679,12 @@ static void lsi_request_cancelled(SCSIRequest *req)
/* Record that data is available for a queued command. Returns zero if
the device was reselected, nonzero if the IO is deferred. */
-static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t len)
+static int lsi_queue_req(LSIState *s, SCSIRequest *req, uint32_t len)
{
-lsi_request *p;
-
-p = lsi_find_by_tag(s, tag);
-if (!p) {
-BADF(IO with unknown tag %d\n, tag);
-return 1;
-}
+lsi_request *p = req-hba_private;
if (p-pending) {
-BADF(Multiple IO pending for tag %d\n, tag);
+BADF(Multiple IO pending for request %p\n, p);
}
p-pending = len;
/* Reselect if waiting for it, or if reselection triggers an IRQ
@@ -743,9 +736,9 @@ static void lsi_transfer_data(SCSIRequest *req, uint32_t
len)
LSIState *s = DO_UPCAST(LSIState, dev.qdev, req-bus-qbus.parent);
int out;
-if (s-waiting == 1 || !s-current || req-tag != s-current-tag ||
+if (s-waiting == 1 || !s-current || req-hba_private != s-current ||
(lsi_irq_on_rsl(s) !(s-scntl1 LSI_SCNTL1_CON))) {
-if (lsi_queue_tag(s, req-tag, len)) {
+if (lsi_queue_req(s, req, len)) {
return;
}
}
@@ -789,7 +782,8 @@ static void lsi_do_command(LSIState *s)
assert(s-current == NULL);
s-current = qemu_mallocz(sizeof(lsi_request));
s-current-tag = s-select_tag;
-s-current-req = scsi_req_new(dev, s-current-tag, s-current_lun);
+s-current-req = scsi_req_new(dev, s-current-tag, s-current_lun,
+ s-current);
n = scsi_req_enqueue(s-current-req, buf);
if (n) {
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index ad6a730..8b1a412 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -131,7 +131,8 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
return res;
}
-SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t
lun)
+SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
+uint32_t lun, void *hba_private)
{
SCSIRequest *req;
@@ -141,14 +142,16 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d,
uint32_t tag, uint32_t l
req-dev = d;
req-tag = tag;
req-lun = lun;
+req-hba_private = hba_private;
req-status = -1;
trace_scsi_req_alloc(req-dev-id, req-lun, req-tag);
return req;
}
-SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun)
+SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
+ void *hba_private)
{
-return d-info-alloc_req(d, tag, lun);
+return d-info-alloc_req(d, tag, lun, hba_private);
}
uint8_t *scsi_req_get_buf(SCSIRequest *req)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a8c7372..c2a99fe 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -81,13 +81,13 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error,
int type);
static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf);
static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t
Re: [PATCH 3/3] megasas: LSI Megaraid SAS emulation
On 07/01/2011 06:42 PM, Alexander Graf wrote: On 01.07.2011, at 17:35, Hannes Reinecke wrote: This patch adds an emulation for the LSI Megaraid SAS 8708EM2 HBA. Have you tried to execute the current version of megasas and actually do something with it? I just booted up openSUSE 11.4 rescue from DVD with a megasas adapter that contained a raw file backed by tmpfs. Creating a partition worked fine, but when running mkfs.ext3 and mounting afterwards, the mount fails saying there is no ext3 on the disk. Sounds like data corruption to me :). I know that this used to work a while back, so it might be a regression recently? It worked here, in the sense that I've booted up an existing openSUSE 11.4 installation. But I wouldn't be surprised if the degradation to use bounce-buffers has some flaws. My guess here is that we have problems when the transfersizes larger as the internal bounce buffer. (I probably should be putting in some more references to 'bounce buffers' here to alert people that using bounce buffers in SCSI is the best way of killing performance) (And no, I will not getting into another dog-fight with Paul B. here. Virtio can do without bounce buffers. AHCI can. So I fail to see why SCSI has to rely on bounce buffers.) But enough of this, Yeah, bugfixing is needed here. I see what I can do. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] megasas: LSI Megaraid SAS emulation
On 07/03/2011 04:36 PM, Paolo Bonzini wrote: On 07/02/2011 03:50 PM, Hannes Reinecke wrote: (And no, I will not getting into another dog-fight with Paul B. here. Virtio can do without bounce buffers. AHCI can. So I fail to see why SCSI has to rely on bounce buffers.) I agree, but I do see why a SCSI device might prefer to rely on bounce buffers for non-I/O commands. This is why in my last RFC series for vmw_pvscsi I let the device choose whether to force a bounce buffer or get an external iovec from the HBA. Yes, sure, for non-I/O commands it's perfectly okay. Most of which will be emulated anyway. It's bounce buffers for I/O which kills performance. But I seem to have missed your last RFC (I'm not reading qemu-devel on a regular basis ...). Care to send me a pointer to it? Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/3] megasas: LSI Megaraid SAS emulation
;
+s-producer_pa = 0;
+s-consumer_pa = 0;
+for (i = 0; i s-fw_cmds; i++) {
+s-frames[i].index = i;
+s-frames[i].context = -1;
+s-frames[i].pa = 0;
+s-frames[i].state = s;
+}
It is not clear to me that all register state is initialized here.
megasas_soft_reset() seems to touch fw_state and intr_mask but will
not be called until mmio_writel(MFI_IDB, MFI_FWINIT_READY). Are there
any missing fields that need to be initialized here?
I was under the impression that we'll be getting a reset after
initialising the device, so this should've been taken care of.
And most of the mmio writes are safe against accidental misuse.
I'll be adding a check to MFI_ODCR0, as this indeed might cause
an error when used uninitialized.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] megasas: LSI Megaraid SAS emulation
On 07/04/2011 12:29 PM, Paolo Bonzini wrote: On 07/04/2011 09:26 AM, Hannes Reinecke wrote: Cool. Exactly what I need. FWIW, feel free to add my 'Acked-by' to it. Any chance of getting them included? I'm not very tied to pvscsi; I just needed an HBA that is not a joke by modern standards :) to play with the SCSI layer. There may be hope that megasas will come before pvscsi or eliminate the need for it altogether. So, feel free to pick up patches 5-8 from that series. Hmm. My goal was actually to get the megasas HBA emulation upstream. When sticking it behind another set of patches chances are not exactly increasing. However, it would certainly make my life easier. That said, note that scsi-generic does *not* support scatter/gather in that series, so you should still make sure the fallback paths work well. :) In pvscsi I added a qdev property to toggle scatter/gather, it was useful for benchmarking. Have to check. My patchset did, so it should be reasonably easy to port that one to your infrastructure. However, I probably will see to fixup the megasas emulation with the current infrastructure, get that in, and then move over to the iovec infrastructure. But if you promise to merge the iovec infrastructure I'm more than willing to fixup the scsi-generic backend. Just say the word. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5][v6] Megasas HBA emulation
Hi all, as Alex Graf reminded me the driver needed some more bugfixing to be done. I've found some issues and also moved the megasas emulation over to the new trace infrastructure. Driver works for me now and a full installation of openSUSE-12.1 works perfectly. I've also included the fixes suggested by Stefan Hajnoczi. And during debugging I've found two minor issues in scsi_disk.c Changes since v5: - scsi-disk: Fixup debugging statement A debugging statement wasn't converted. Do so now. - scsi-disk: Mask out serial number EVPD The 'serial' parameter to scsi-disk is optional. So if it's not set we should mask it out in the list of supported EVPD pages and not return '0' here. - megasas: Use tracing infrastructure instead of DPRINTF - megasas: Use new PCI infrastructure - megasas: Check for iovec mapping failure cpu_map_physical_memory() might fail, so we need to check for it when mapping iovecs. - megasas: Trace scsi buffer overflow The transfer length as specified in the SCSI command might disagree with the length of the iovec. We should be tracing these issues. - megasas: Reset frames after init firmware When receiving an INIT FIRMWARE command we need reset all frames, otherwise some frames might point to invalid memory. Chances since v4: - iov: Update parameter usage in iov_(to|from)_buf() Updated description for the first patch and clarified the usage Renamed arguments for io_XXX for clarification - scsi: Add 'hba_private' to SCSIRequest Kept 'tag' for tracing and just add 'hba_private' as an additional field as per request from Paolo - megasas: checkpatch.pl fixes and update to work with the changed interface in scsi_req_new(). Also included the suggested fixes from Alex. Hannes Reinecke (5): iov: Update parameter usage in iov_(to|from)_buf() scsi: Add 'hba_private' to SCSIRequest scsi-disk: Fixup debugging statement scsi-disk: Mask out serial number EVPD megasas: LSI Megaraid SAS emulation Makefile.objs |1 + default-configs/pci.mak |1 + hw/esp.c|2 +- hw/lsi53c895a.c | 22 +- hw/megasas.c| 1950 +++ hw/mfi.h| 1197 + hw/pci_ids.h|3 +- hw/scsi-bus.c |9 +- hw/scsi-disk.c | 21 +- hw/scsi-generic.c |5 +- hw/scsi.h | 10 +- hw/spapr_vscsi.c| 29 +- hw/usb-msd.c|9 +- hw/virtio-net.c |2 +- hw/virtio-serial-bus.c |2 +- iov.c | 49 +- iov.h | 10 +- trace-events| 71 ++ 18 files changed, 3306 insertions(+), 87 deletions(-) create mode 100644 hw/megasas.c create mode 100644 hw/mfi.h -- 1.7.3.4 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] iov: Update parameter usage in iov_(to|from)_buf()
iov_to_buf() has an 'offset' parameter, iov_from_buf() hasn't.
This patch adds the missing parameter to iov_from_buf().
It also renames the 'offset' parameter to 'iov_off' to
emphasize it's the offset into the iovec and not the buffer.
Signed-off-by: Hannes Reinecke h...@suse.de
Acked-by: Alexander Graf ag...@suse.de
---
hw/virtio-net.c|2 +-
hw/virtio-serial-bus.c |2 +-
iov.c | 49 ++-
iov.h | 10
4 files changed, 34 insertions(+), 29 deletions(-)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 6997e02..a32cc01 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -657,7 +657,7 @@ static ssize_t virtio_net_receive(VLANClientState *nc,
const uint8_t *buf, size_
/* copy in packet. ugh */
len = iov_from_buf(sg, elem.in_num,
- buf + offset, size - offset);
+ buf + offset, 0, size - offset);
total += len;
offset += len;
/* If buffers can't be merged, at this point we
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 7f6db7b..53c58d0 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -103,7 +103,7 @@ static size_t write_to_port(VirtIOSerialPort *port,
}
len = iov_from_buf(elem.in_sg, elem.in_num,
- buf + offset, size - offset);
+ buf + offset, 0, size - offset);
offset += len;
virtqueue_push(vq, elem, len);
diff --git a/iov.c b/iov.c
index 588cd04..1e02791 100644
--- a/iov.c
+++ b/iov.c
@@ -14,56 +14,61 @@
#include iov.h
-size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
-const void *buf, size_t size)
+size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt,
+const void *buf, size_t iov_off, size_t size)
{
-size_t offset;
+size_t iovec_off, buf_off;
unsigned int i;
-offset = 0;
-for (i = 0; offset size i iovcnt; i++) {
-size_t len;
+iovec_off = 0;
+buf_off = 0;
+for (i = 0; i iov_cnt size; i++) {
+if (iov_off (iovec_off + iov[i].iov_len)) {
+size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off, size);
-len = MIN(iov[i].iov_len, size - offset);
+memcpy(iov[i].iov_base + (iov_off - iovec_off), buf + buf_off,
len);
-memcpy(iov[i].iov_base, buf + offset, len);
-offset += len;
+buf_off += len;
+iov_off += len;
+size -= len;
+}
+iovec_off += iov[i].iov_len;
}
-return offset;
+return buf_off;
}
-size_t iov_to_buf(const struct iovec *iov, const unsigned int iovcnt,
- void *buf, size_t offset, size_t size)
+size_t iov_to_buf(const struct iovec *iov, const unsigned int iov_cnt,
+ void *buf, size_t iov_off, size_t size)
{
uint8_t *ptr;
-size_t iov_off, buf_off;
+size_t iovec_off, buf_off;
unsigned int i;
ptr = buf;
-iov_off = 0;
+iovec_off = 0;
buf_off = 0;
-for (i = 0; i iovcnt size; i++) {
-if (offset (iov_off + iov[i].iov_len)) {
-size_t len = MIN((iov_off + iov[i].iov_len) - offset , size);
+for (i = 0; i iov_cnt size; i++) {
+if (iov_off (iovec_off + iov[i].iov_len)) {
+size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off , size);
-memcpy(ptr + buf_off, iov[i].iov_base + (offset - iov_off), len);
+memcpy(ptr + buf_off, iov[i].iov_base + (iov_off - iovec_off),
len);
buf_off += len;
-offset += len;
+iov_off += len;
size -= len;
}
-iov_off += iov[i].iov_len;
+iovec_off += iov[i].iov_len;
}
return buf_off;
}
-size_t iov_size(const struct iovec *iov, const unsigned int iovcnt)
+size_t iov_size(const struct iovec *iov, const unsigned int iov_cnt)
{
size_t len;
unsigned int i;
len = 0;
-for (i = 0; i iovcnt; i++) {
+for (i = 0; i iov_cnt; i++) {
len += iov[i].iov_len;
}
return len;
diff --git a/iov.h b/iov.h
index 60a8547..110f67a 100644
--- a/iov.h
+++ b/iov.h
@@ -12,8 +12,8 @@
#include qemu-common.h
-size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
-const void *buf, size_t size);
-size_t iov_to_buf(const struct iovec *iov, const unsigned int iovcnt,
- void *buf, size_t offset, size_t size);
-size_t iov_size(const struct iovec *iov, const unsigned int iovcnt);
+size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt,
+const void *buf, size_t iov_off, size_t size);
+size_t iov_to_buf(const struct iovec *iov, const unsigned int iov_cnt,
+ void *buf, size_t iov_off, size_t size);
+size_t iov_size(const struct iovec *iov, const unsigned int iov_cnt);
--
1.7.3.4
[PATCH 2/5] scsi: Add 'hba_private' to SCSIRequest
'tag' is just an abstraction to identify the command
from the driver. So we should make that explicit by
replacing 'tag' with a driver-defined pointer 'hba_private'.
This saves the lookup for driver handling several commands
in parallel.
'tag' is still being kept for tracing purposes.
Signed-off-by: Hannes Reinecke h...@suse.de
Acked-by: Paolo Bonzini pbonz...@redhat.com
---
hw/esp.c |2 +-
hw/lsi53c895a.c | 22 --
hw/scsi-bus.c |9 ++---
hw/scsi-disk.c|4 ++--
hw/scsi-generic.c |5 +++--
hw/scsi.h | 10 +++---
hw/spapr_vscsi.c | 29 +
hw/usb-msd.c |9 +
8 files changed, 37 insertions(+), 53 deletions(-)
diff --git a/hw/esp.c b/hw/esp.c
index 8e95672..69209bd 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -244,7 +244,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t
busid)
DPRINTF(do_busid_cmd: busid 0x%x\n, busid);
lun = busid 7;
-s-current_req = scsi_req_new(s-current_dev, 0, lun);
+s-current_req = scsi_req_new(s-current_dev, 0, lun, NULL);
datalen = scsi_req_enqueue(s-current_req, buf);
s-ti_size = datalen;
if (datalen != 0) {
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 940b43a..69eec1d 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -661,7 +661,7 @@ static lsi_request *lsi_find_by_tag(LSIState *s, uint32_t
tag)
static void lsi_request_cancelled(SCSIRequest *req)
{
LSIState *s = DO_UPCAST(LSIState, dev.qdev, req-bus-qbus.parent);
-lsi_request *p;
+lsi_request *p = req-hba_private;
if (s-current req == s-current-req) {
scsi_req_unref(req);
@@ -670,7 +670,6 @@ static void lsi_request_cancelled(SCSIRequest *req)
return;
}
-p = lsi_find_by_tag(s, req-tag);
if (p) {
QTAILQ_REMOVE(s-queue, p, next);
scsi_req_unref(req);
@@ -680,18 +679,12 @@ static void lsi_request_cancelled(SCSIRequest *req)
/* Record that data is available for a queued command. Returns zero if
the device was reselected, nonzero if the IO is deferred. */
-static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t len)
+static int lsi_queue_req(LSIState *s, SCSIRequest *req, uint32_t len)
{
-lsi_request *p;
-
-p = lsi_find_by_tag(s, tag);
-if (!p) {
-BADF(IO with unknown tag %d\n, tag);
-return 1;
-}
+lsi_request *p = req-hba_private;
if (p-pending) {
-BADF(Multiple IO pending for tag %d\n, tag);
+BADF(Multiple IO pending for request %p\n, p);
}
p-pending = len;
/* Reselect if waiting for it, or if reselection triggers an IRQ
@@ -743,9 +736,9 @@ static void lsi_transfer_data(SCSIRequest *req, uint32_t
len)
LSIState *s = DO_UPCAST(LSIState, dev.qdev, req-bus-qbus.parent);
int out;
-if (s-waiting == 1 || !s-current || req-tag != s-current-tag ||
+if (s-waiting == 1 || !s-current || req-hba_private != s-current ||
(lsi_irq_on_rsl(s) !(s-scntl1 LSI_SCNTL1_CON))) {
-if (lsi_queue_tag(s, req-tag, len)) {
+if (lsi_queue_req(s, req, len)) {
return;
}
}
@@ -789,7 +782,8 @@ static void lsi_do_command(LSIState *s)
assert(s-current == NULL);
s-current = qemu_mallocz(sizeof(lsi_request));
s-current-tag = s-select_tag;
-s-current-req = scsi_req_new(dev, s-current-tag, s-current_lun);
+s-current-req = scsi_req_new(dev, s-current-tag, s-current_lun,
+ s-current);
n = scsi_req_enqueue(s-current-req, buf);
if (n) {
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index ad6a730..8b1a412 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -131,7 +131,8 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
return res;
}
-SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t
lun)
+SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
+uint32_t lun, void *hba_private)
{
SCSIRequest *req;
@@ -141,14 +142,16 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d,
uint32_t tag, uint32_t l
req-dev = d;
req-tag = tag;
req-lun = lun;
+req-hba_private = hba_private;
req-status = -1;
trace_scsi_req_alloc(req-dev-id, req-lun, req-tag);
return req;
}
-SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun)
+SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
+ void *hba_private)
{
-return d-info-alloc_req(d, tag, lun);
+return d-info-alloc_req(d, tag, lun, hba_private);
}
uint8_t *scsi_req_get_buf(SCSIRequest *req)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a8c7372..c2a99fe 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -81,13 +81,13 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error,
int type);
static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf);
static
[PATCH 3/5] scsi-disk: Fixup debugging statement
A debugging statement wasn't converted to the new interface.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/scsi-disk.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index c2a99fe..5804662 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1007,7 +1007,7 @@ static int32_t scsi_send_command(SCSIRequest *req,
uint8_t *buf)
command = buf[0];
outbuf = (uint8_t *)r-iov.iov_base;
-DPRINTF(Command: lun=%d tag=0x%x data=0x%02x, lun, tag, buf[0]);
+DPRINTF(Command: lun=%d tag=0x%x data=0x%02x, req-lun, req-tag,
buf[0]);
if (scsi_req_parse(r-req, buf) != 0) {
BADF(Unsupported command length, command %x\n, command);
--
1.7.3.4
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] scsi-disk: Mask out serial number EVPD
If the serial number is not set we should mask it out in the
list of supported VPD pages and mark it as not supported.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/scsi-disk.c | 15 ---
1 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 5804662..05d14ab 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -398,7 +398,8 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
uint8_t *outbuf)
buffer size %zd\n, req-cmd.xfer);
pages = buflen++;
outbuf[buflen++] = 0x00; // list of supported pages (this page)
-outbuf[buflen++] = 0x80; // unit serial number
+if (s-serial)
+outbuf[buflen++] = 0x80; // unit serial number
outbuf[buflen++] = 0x83; // device identification
if (s-drive_kind == SCSI_HD) {
outbuf[buflen++] = 0xb0; // block limits
@@ -409,8 +410,14 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
uint8_t *outbuf)
}
case 0x80: /* Device serial number, optional */
{
-int l = strlen(s-serial);
+int l;
+if (!s-serial) {
+DPRINTF(Inquiry (EVPD[Serial number] not supported\n);
+return -1;
+}
+
+l = strlen(s-serial);
if (l req-cmd.xfer)
l = req-cmd.xfer;
if (l 20)
@@ -1203,7 +1210,9 @@ static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind
kind)
if (!s-serial) {
/* try to fall back to value set with legacy -drive serial=... */
dinfo = drive_get_by_blockdev(s-bs);
-s-serial = qemu_strdup(*dinfo-serial ? dinfo-serial : 0);
+if (*dinfo-serial) {
+s-serial = qemu_strdup(dinfo-serial);
+}
}
if (!s-version) {
--
1.7.3.4
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 5/5] megasas: LSI Megaraid SAS emulation
On 07/05/2011 05:21 PM, Stefan Hajnoczi wrote:
On Tue, Jul 5, 2011 at 12:03 PM, Hannes Reineckeh...@suse.de wrote:
+static void megasas_unmap_sgl(struct megasas_cmd_t *cmd)
+{
+uint16_t flags = le16_to_cpu(cmd-frame-header.flags);
+int i, is_write = (flags MFI_FRAME_DIR_WRITE) ? 1 : 0;
+
+for (i = 0; i cmd-frame-header.sge_count; i++) {
+cpu_physical_memory_unmap(cmd-iov[i].iov_base, cmd-iov[i].iov_len,
+ is_write, cmd-iov[i].iov_len);
+}
We cannot map control structures from guest memory and treating them
as valid request state later on.
Yes, I've been working on that one already.
What I'll be doing is to read in the sge count during 'map_sgl' and
store this value internally (in -iov_cnt). And during unmap I'll be
using this value instead of the frame-provided one.
That way we'll be checking the sge_count field only once when we
slurp in the entire frame.
A malicious guest can issue the request, then change the fields the
control structure while QEMU is processing the I/O, and then this
function will execute with is_write/sge_count no longer the same as
when the request started.
Good practice would be to copy in any request state needed instead of
reaching into guest memory at later points of the request lifecycle.
This way a malicious guest can never cause QEMU to crash or do
something due to inconsistent state.
See above, that's what I'll be doing.
The particular problem I see here is starting the request with
sge_count=1 and then setting it to sge_count=255. We will perform
invalid iov[] accesses.
Thanks for the hint. Will be fixing it up.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] scsi fixes
Hi all, these are some fixes I found during debugging my megasas HBA emulation. This time I've sent them as a separate patchset for inclusion. All of them have been acked, so please apply. Hannes Reinecke (4): iov: Update parameter usage in iov_(to|from)_buf() scsi: Add 'hba_private' to SCSIRequest scsi-disk: Fixup debugging statement scsi-disk: Mask out serial number EVPD hw/esp.c |2 +- hw/lsi53c895a.c| 22 +++- hw/scsi-bus.c |9 +-- hw/scsi-disk.c | 21 ++- hw/scsi-generic.c |5 ++- hw/scsi.h | 10 ++-- hw/spapr_vscsi.c | 29 --- hw/usb-msd.c |9 +--- hw/virtio-net.c|2 +- hw/virtio-serial-bus.c |2 +- iov.c | 49 ++- iov.h | 10 12 files changed, 84 insertions(+), 86 deletions(-) -- 1.7.3.4 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] iov: Update parameter usage in iov_(to|from)_buf()
iov_to_buf() has an 'offset' parameter, iov_from_buf() hasn't.
This patch adds the missing parameter to iov_from_buf().
It also renames the 'offset' parameter to 'iov_off' to
emphasize it's the offset into the iovec and not the buffer.
Signed-off-by: Hannes Reinecke h...@suse.de
Acked-by: Alexander Graf ag...@suse.de
---
hw/virtio-net.c|2 +-
hw/virtio-serial-bus.c |2 +-
iov.c | 49 ++-
iov.h | 10
4 files changed, 34 insertions(+), 29 deletions(-)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 6997e02..a32cc01 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -657,7 +657,7 @@ static ssize_t virtio_net_receive(VLANClientState *nc,
const uint8_t *buf, size_
/* copy in packet. ugh */
len = iov_from_buf(sg, elem.in_num,
- buf + offset, size - offset);
+ buf + offset, 0, size - offset);
total += len;
offset += len;
/* If buffers can't be merged, at this point we
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 7f6db7b..53c58d0 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -103,7 +103,7 @@ static size_t write_to_port(VirtIOSerialPort *port,
}
len = iov_from_buf(elem.in_sg, elem.in_num,
- buf + offset, size - offset);
+ buf + offset, 0, size - offset);
offset += len;
virtqueue_push(vq, elem, len);
diff --git a/iov.c b/iov.c
index 588cd04..1e02791 100644
--- a/iov.c
+++ b/iov.c
@@ -14,56 +14,61 @@
#include iov.h
-size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
-const void *buf, size_t size)
+size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt,
+const void *buf, size_t iov_off, size_t size)
{
-size_t offset;
+size_t iovec_off, buf_off;
unsigned int i;
-offset = 0;
-for (i = 0; offset size i iovcnt; i++) {
-size_t len;
+iovec_off = 0;
+buf_off = 0;
+for (i = 0; i iov_cnt size; i++) {
+if (iov_off (iovec_off + iov[i].iov_len)) {
+size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off, size);
-len = MIN(iov[i].iov_len, size - offset);
+memcpy(iov[i].iov_base + (iov_off - iovec_off), buf + buf_off,
len);
-memcpy(iov[i].iov_base, buf + offset, len);
-offset += len;
+buf_off += len;
+iov_off += len;
+size -= len;
+}
+iovec_off += iov[i].iov_len;
}
-return offset;
+return buf_off;
}
-size_t iov_to_buf(const struct iovec *iov, const unsigned int iovcnt,
- void *buf, size_t offset, size_t size)
+size_t iov_to_buf(const struct iovec *iov, const unsigned int iov_cnt,
+ void *buf, size_t iov_off, size_t size)
{
uint8_t *ptr;
-size_t iov_off, buf_off;
+size_t iovec_off, buf_off;
unsigned int i;
ptr = buf;
-iov_off = 0;
+iovec_off = 0;
buf_off = 0;
-for (i = 0; i iovcnt size; i++) {
-if (offset (iov_off + iov[i].iov_len)) {
-size_t len = MIN((iov_off + iov[i].iov_len) - offset , size);
+for (i = 0; i iov_cnt size; i++) {
+if (iov_off (iovec_off + iov[i].iov_len)) {
+size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off , size);
-memcpy(ptr + buf_off, iov[i].iov_base + (offset - iov_off), len);
+memcpy(ptr + buf_off, iov[i].iov_base + (iov_off - iovec_off),
len);
buf_off += len;
-offset += len;
+iov_off += len;
size -= len;
}
-iov_off += iov[i].iov_len;
+iovec_off += iov[i].iov_len;
}
return buf_off;
}
-size_t iov_size(const struct iovec *iov, const unsigned int iovcnt)
+size_t iov_size(const struct iovec *iov, const unsigned int iov_cnt)
{
size_t len;
unsigned int i;
len = 0;
-for (i = 0; i iovcnt; i++) {
+for (i = 0; i iov_cnt; i++) {
len += iov[i].iov_len;
}
return len;
diff --git a/iov.h b/iov.h
index 60a8547..110f67a 100644
--- a/iov.h
+++ b/iov.h
@@ -12,8 +12,8 @@
#include qemu-common.h
-size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
-const void *buf, size_t size);
-size_t iov_to_buf(const struct iovec *iov, const unsigned int iovcnt,
- void *buf, size_t offset, size_t size);
-size_t iov_size(const struct iovec *iov, const unsigned int iovcnt);
+size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt,
+const void *buf, size_t iov_off, size_t size);
+size_t iov_to_buf(const struct iovec *iov, const unsigned int iov_cnt,
+ void *buf, size_t iov_off, size_t size);
+size_t iov_size(const struct iovec *iov, const unsigned int iov_cnt);
--
1.7.3.4
[PATCH 2/4] scsi: Add 'hba_private' to SCSIRequest
'tag' is just an abstraction to identify the command
from the driver. So we should make that explicit by
replacing 'tag' with a driver-defined pointer 'hba_private'.
This saves the lookup for driver handling several commands
in parallel.
'tag' is still being kept for tracing purposes.
Signed-off-by: Hannes Reinecke h...@suse.de
Acked-by: Paolo Bonzini pbonz...@redhat.com
---
hw/esp.c |2 +-
hw/lsi53c895a.c | 22 --
hw/scsi-bus.c |9 ++---
hw/scsi-disk.c|4 ++--
hw/scsi-generic.c |5 +++--
hw/scsi.h | 10 +++---
hw/spapr_vscsi.c | 29 +
hw/usb-msd.c |9 +
8 files changed, 37 insertions(+), 53 deletions(-)
diff --git a/hw/esp.c b/hw/esp.c
index 8e95672..69209bd 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -244,7 +244,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t
busid)
DPRINTF(do_busid_cmd: busid 0x%x\n, busid);
lun = busid 7;
-s-current_req = scsi_req_new(s-current_dev, 0, lun);
+s-current_req = scsi_req_new(s-current_dev, 0, lun, NULL);
datalen = scsi_req_enqueue(s-current_req, buf);
s-ti_size = datalen;
if (datalen != 0) {
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 940b43a..69eec1d 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -661,7 +661,7 @@ static lsi_request *lsi_find_by_tag(LSIState *s, uint32_t
tag)
static void lsi_request_cancelled(SCSIRequest *req)
{
LSIState *s = DO_UPCAST(LSIState, dev.qdev, req-bus-qbus.parent);
-lsi_request *p;
+lsi_request *p = req-hba_private;
if (s-current req == s-current-req) {
scsi_req_unref(req);
@@ -670,7 +670,6 @@ static void lsi_request_cancelled(SCSIRequest *req)
return;
}
-p = lsi_find_by_tag(s, req-tag);
if (p) {
QTAILQ_REMOVE(s-queue, p, next);
scsi_req_unref(req);
@@ -680,18 +679,12 @@ static void lsi_request_cancelled(SCSIRequest *req)
/* Record that data is available for a queued command. Returns zero if
the device was reselected, nonzero if the IO is deferred. */
-static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t len)
+static int lsi_queue_req(LSIState *s, SCSIRequest *req, uint32_t len)
{
-lsi_request *p;
-
-p = lsi_find_by_tag(s, tag);
-if (!p) {
-BADF(IO with unknown tag %d\n, tag);
-return 1;
-}
+lsi_request *p = req-hba_private;
if (p-pending) {
-BADF(Multiple IO pending for tag %d\n, tag);
+BADF(Multiple IO pending for request %p\n, p);
}
p-pending = len;
/* Reselect if waiting for it, or if reselection triggers an IRQ
@@ -743,9 +736,9 @@ static void lsi_transfer_data(SCSIRequest *req, uint32_t
len)
LSIState *s = DO_UPCAST(LSIState, dev.qdev, req-bus-qbus.parent);
int out;
-if (s-waiting == 1 || !s-current || req-tag != s-current-tag ||
+if (s-waiting == 1 || !s-current || req-hba_private != s-current ||
(lsi_irq_on_rsl(s) !(s-scntl1 LSI_SCNTL1_CON))) {
-if (lsi_queue_tag(s, req-tag, len)) {
+if (lsi_queue_req(s, req, len)) {
return;
}
}
@@ -789,7 +782,8 @@ static void lsi_do_command(LSIState *s)
assert(s-current == NULL);
s-current = qemu_mallocz(sizeof(lsi_request));
s-current-tag = s-select_tag;
-s-current-req = scsi_req_new(dev, s-current-tag, s-current_lun);
+s-current-req = scsi_req_new(dev, s-current-tag, s-current_lun,
+ s-current);
n = scsi_req_enqueue(s-current-req, buf);
if (n) {
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index ad6a730..8b1a412 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -131,7 +131,8 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
return res;
}
-SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t
lun)
+SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
+uint32_t lun, void *hba_private)
{
SCSIRequest *req;
@@ -141,14 +142,16 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d,
uint32_t tag, uint32_t l
req-dev = d;
req-tag = tag;
req-lun = lun;
+req-hba_private = hba_private;
req-status = -1;
trace_scsi_req_alloc(req-dev-id, req-lun, req-tag);
return req;
}
-SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun)
+SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
+ void *hba_private)
{
-return d-info-alloc_req(d, tag, lun);
+return d-info-alloc_req(d, tag, lun, hba_private);
}
uint8_t *scsi_req_get_buf(SCSIRequest *req)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a8c7372..c2a99fe 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -81,13 +81,13 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error,
int type);
static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf);
static
[PATCH 3/4] scsi-disk: Fixup debugging statement
A debugging statement wasn't converted to the new interface.
Signed-off-by: Hannes Reinecke h...@suse.de
Acked-by: Paolo Bonzini pbonz...@redhat.com
---
hw/scsi-disk.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index c2a99fe..5804662 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1007,7 +1007,7 @@ static int32_t scsi_send_command(SCSIRequest *req,
uint8_t *buf)
command = buf[0];
outbuf = (uint8_t *)r-iov.iov_base;
-DPRINTF(Command: lun=%d tag=0x%x data=0x%02x, lun, tag, buf[0]);
+DPRINTF(Command: lun=%d tag=0x%x data=0x%02x, req-lun, req-tag,
buf[0]);
if (scsi_req_parse(r-req, buf) != 0) {
BADF(Unsupported command length, command %x\n, command);
--
1.7.3.4
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] scsi-disk: Mask out serial number EVPD
If the serial number is not set we should mask it out in the
list of supported VPD pages and mark it as not supported.
Signed-off-by: Hannes Reinecke h...@suse.de
Acked-by: Paolo Bonzini pbonz...@redhat.com
---
hw/scsi-disk.c | 15 ---
1 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 5804662..05d14ab 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -398,7 +398,8 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
uint8_t *outbuf)
buffer size %zd\n, req-cmd.xfer);
pages = buflen++;
outbuf[buflen++] = 0x00; // list of supported pages (this page)
-outbuf[buflen++] = 0x80; // unit serial number
+if (s-serial)
+outbuf[buflen++] = 0x80; // unit serial number
outbuf[buflen++] = 0x83; // device identification
if (s-drive_kind == SCSI_HD) {
outbuf[buflen++] = 0xb0; // block limits
@@ -409,8 +410,14 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
uint8_t *outbuf)
}
case 0x80: /* Device serial number, optional */
{
-int l = strlen(s-serial);
+int l;
+if (!s-serial) {
+DPRINTF(Inquiry (EVPD[Serial number] not supported\n);
+return -1;
+}
+
+l = strlen(s-serial);
if (l req-cmd.xfer)
l = req-cmd.xfer;
if (l 20)
@@ -1203,7 +1210,9 @@ static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind
kind)
if (!s-serial) {
/* try to fall back to value set with legacy -drive serial=... */
dinfo = drive_get_by_blockdev(s-bs);
-s-serial = qemu_strdup(*dinfo-serial ? dinfo-serial : 0);
+if (*dinfo-serial) {
+s-serial = qemu_strdup(dinfo-serial);
+}
}
if (!s-version) {
--
1.7.3.4
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] scsi fixes
On 07/11/2011 03:42 PM, Kevin Wolf wrote: Am 11.07.2011 15:34, schrieb Stefan Hajnoczi: On Mon, Jul 11, 2011 at 2:02 PM, Hannes Reineckeh...@suse.de wrote: Hi all, these are some fixes I found during debugging my megasas HBA emulation. This time I've sent them as a separate patchset for inclusion. All of them have been acked, so please apply. Are SCSI patches going through Kevin's tree? If not, perhaps Paolo or I should keep a tree and start doing some sanity testing on the subsystem in the future. As long as we don't have a SCSI maintainer, I'm going to pick them up for the block tree when they have receive some review. Doesn't mean that nobody should be doing sanity testing, of course. If anyone wants to take care of picking up and reviewing all SCSI patches, I'm also happy to pull from a separate tree. Patches have already been reviewed and tested, in conjunction with my megasas HBA emulation patchset. This is just a repost as a separate patchset to get them in. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Check for supported SCSI commands
Markus Armbruster pointed out that not every SCSI command is supported for a given device type. Based on his patch this series cleans up the SCSI device type and adds a check for supported commands. Hannes Reinecke (3): scsi: Sanitize command definitions scsi-disk: Remove drive_kind scsi-disk: Check for supported commands hw/scsi-bus.c | 71 ++--- hw/scsi-defs.h| 60 -- hw/scsi-disk.c| 83 hw/scsi-generic.c |2 +- 4 files changed, 118 insertions(+), 98 deletions(-) -- 1.7.3.4 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] scsi: Sanitize command definitions
Adding some missing command definitions and update the existing ones.
No functional change.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/scsi-bus.c | 71 ++--
hw/scsi-defs.h| 60
hw/scsi-disk.c| 29 -
hw/scsi-generic.c |2 +-
4 files changed, 91 insertions(+), 71 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 8b1a412..24a1298 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -232,24 +232,24 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
case RELEASE:
case ERASE:
case ALLOW_MEDIUM_REMOVAL:
-case VERIFY:
+case VERIFY_10:
case SEEK_10:
case SYNCHRONIZE_CACHE:
case LOCK_UNLOCK_CACHE:
case LOAD_UNLOAD:
case SET_CD_SPEED:
case SET_LIMITS:
-case WRITE_LONG:
+case WRITE_LONG_10:
case MOVE_MEDIUM:
case UPDATE_BLOCK:
req-cmd.xfer = 0;
break;
case MODE_SENSE:
break;
-case WRITE_SAME:
+case WRITE_SAME_10:
req-cmd.xfer = 1;
break;
-case READ_CAPACITY:
+case READ_CAPACITY_10:
req-cmd.xfer = 8;
break;
case READ_BLOCK_LIMITS:
@@ -265,7 +265,7 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
req-cmd.xfer *= 8;
break;
case WRITE_10:
-case WRITE_VERIFY:
+case WRITE_VERIFY_10:
case WRITE_6:
case WRITE_12:
case WRITE_VERIFY_12:
@@ -325,7 +325,7 @@ static void scsi_req_xfer_mode(SCSIRequest *req)
switch (req-cmd.buf[0]) {
case WRITE_6:
case WRITE_10:
-case WRITE_VERIFY:
+case WRITE_VERIFY_10:
case WRITE_12:
case WRITE_VERIFY_12:
case WRITE_16:
@@ -345,15 +345,13 @@ static void scsi_req_xfer_mode(SCSIRequest *req)
case SEARCH_HIGH:
case SEARCH_LOW:
case UPDATE_BLOCK:
-case WRITE_LONG:
-case WRITE_SAME:
+case WRITE_LONG_10:
+case WRITE_SAME_10:
case SEARCH_HIGH_12:
case SEARCH_EQUAL_12:
case SEARCH_LOW_12:
-case SET_WINDOW:
case MEDIUM_SCAN:
case SEND_VOLUME_TAG:
-case WRITE_LONG_2:
case PERSISTENT_RESERVE_OUT:
case MAINTENANCE_OUT:
req-cmd.mode = SCSI_XFER_TO_DEV;
@@ -517,8 +515,7 @@ static const char *scsi_command_name(uint8_t cmd)
{
static const char *names[] = {
[ TEST_UNIT_READY ] = TEST_UNIT_READY,
-[ REZERO_UNIT ] = REZERO_UNIT,
-/* REWIND and REZERO_UNIT use the same operation code */
+[ REWIND ] = REWIND,
[ REQUEST_SENSE] = REQUEST_SENSE,
[ FORMAT_UNIT ] = FORMAT_UNIT,
[ READ_BLOCK_LIMITS] = READ_BLOCK_LIMITS,
@@ -543,14 +540,13 @@ static const char *scsi_command_name(uint8_t cmd)
[ RECEIVE_DIAGNOSTIC ] = RECEIVE_DIAGNOSTIC,
[ SEND_DIAGNOSTIC ] = SEND_DIAGNOSTIC,
[ ALLOW_MEDIUM_REMOVAL ] = ALLOW_MEDIUM_REMOVAL,
-
[ SET_WINDOW ] = SET_WINDOW,
-[ READ_CAPACITY] = READ_CAPACITY,
+[ READ_CAPACITY_10 ] = READ_CAPACITY_10,
[ READ_10 ] = READ_10,
[ WRITE_10 ] = WRITE_10,
[ SEEK_10 ] = SEEK_10,
-[ WRITE_VERIFY ] = WRITE_VERIFY,
-[ VERIFY ] = VERIFY,
+[ WRITE_VERIFY_10 ] = WRITE_VERIFY_10,
+[ VERIFY_10] = VERIFY_10,
[ SEARCH_HIGH ] = SEARCH_HIGH,
[ SEARCH_EQUAL ] = SEARCH_EQUAL,
[ SEARCH_LOW ] = SEARCH_LOW,
@@ -566,11 +562,14 @@ static const char *scsi_command_name(uint8_t cmd)
[ WRITE_BUFFER ] = WRITE_BUFFER,
[ READ_BUFFER ] = READ_BUFFER,
[ UPDATE_BLOCK ] = UPDATE_BLOCK,
-[ READ_LONG] = READ_LONG,
-[ WRITE_LONG ] = WRITE_LONG,
+[ READ_LONG_10 ] = READ_LONG_10,
+[ WRITE_LONG_10] = WRITE_LONG_10,
[ CHANGE_DEFINITION] = CHANGE_DEFINITION,
-[ WRITE_SAME ] = WRITE_SAME,
+[ WRITE_SAME_10] = WRITE_SAME_10,
+[ UNMAP] = UNMAP,
[ READ_TOC ] = READ_TOC,
+[ REPORT_DENSITY_SUPPORT ] = REPORT_DENSITY_SUPPORT,
+[ GET_CONFIGURATION] = GET_CONFIGURATION,
[ LOG_SELECT ] = LOG_SELECT,
[ LOG_SENSE] = LOG_SENSE,
[ MODE_SELECT_10 ] = MODE_SELECT_10,
@@ -579,27 +578,39 @@ static const char *scsi_command_name(uint8_t cmd)
[ MODE_SENSE_10] = MODE_SENSE_10,
[ PERSISTENT_RESERVE_IN] = PERSISTENT_RESERVE_IN,
[ PERSISTENT_RESERVE_OUT ] = PERSISTENT_RESERVE_OUT,
+[ WRITE_FILEMARKS_16
[PATCH 2/3] scsi-disk: Remove drive_kind
Instead of using our own type structure we can be using the
SCSI type from the parent device.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/scsi-disk.c | 156 ++--
1 files changed, 129 insertions(+), 27 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 2f7e0c9..8ad90c0 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -74,7 +74,6 @@ struct SCSIDiskState
char *version;
char *serial;
SCSISense sense;
-SCSIDriveKind drive_kind;
};
static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
@@ -362,13 +361,107 @@ static int scsi_get_sense(SCSIRequest *req, uint8_t
*outbuf, int len)
return scsi_build_sense(s-sense, outbuf, len, len 14);
}
+#define GENERIC_CMD (uint32_t)0x
+#define DISK_CMD (1u TYPE_DISK)
+#define TAPE_CMD (1u TYPE_TAPE)
+#define PRINTER_CMD (1u TYPE_PRINTER)
+#define PROCESSOR_CMD (1u TYPE_PROCESSOR)
+#define WORM_CMD (1u TYPE_WORM)
+#define ROM_CMD (1u TYPE_ROM)
+#define SCANNER_CMD (1u TYPE_SCANNER)
+#define MOD_CMD (1u TYPE_MOD)
+#define MEDIUM_CHANGER_CMD (1u TYPE_MEDIUM_CHANGER)
+#define ARRAY_CMD (1u TYPE_STORAGE_ARRAY)
+#define ENCLOSURE_CMD (1u TYPE_ENCLOSURE)
+#define RBC_CMD (1u TYPE_RBC)
+#define OSD_CMD (1u TYPE_OSD)
+
+#define NO_ROM_CMD (GENERIC_CMD | ~ROM_CMD)
+
+uint32_t scsi_cmd_table[0x100] = {
+[TEST_UNIT_READY] = GENERIC_CMD,
+[REWIND]= TAPE_CMD,
+[REQUEST_SENSE] = GENERIC_CMD,
+[FORMAT_UNIT] = DISK_CMD|ROM_CMD,
+[READ_BLOCK_LIMITS] = TAPE_CMD,
+[REASSIGN_BLOCKS] = DISK_CMD|WORM_CMD|MOD_CMD,
+[READ_6]= DISK_CMD|TAPE_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_6] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD,
+[READ_REVERSE] = TAPE_CMD,
+[WRITE_FILEMARKS] = TAPE_CMD,
+[SPACE] = TAPE_CMD,
+[INQUIRY] = GENERIC_CMD,
+[MODE_SELECT] = GENERIC_CMD,
+[RESERVE] = TAPE_CMD|PRINTER_CMD,
+[RELEASE] = TAPE_CMD|PRINTER_CMD,
+[ERASE] = TAPE_CMD,
+[MODE_SENSE]= GENERIC_CMD,
+[START_STOP]= GENERIC_CMD,
+[RECEIVE_DIAGNOSTIC]= GENERIC_CMD,
+[SEND_DIAGNOSTIC] = GENERIC_CMD,
+[ALLOW_MEDIUM_REMOVAL] = GENERIC_CMD,
+[READ_CAPACITY_10] = DISK_CMD|WORM_CMD|MOD_CMD,
+[READ_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[SEEK_10] = TAPE_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_VERIFY_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[VERIFY_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[READ_POSITION] = TAPE_CMD,
+[SYNCHRONIZE_CACHE] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD|RBC_CMD,
+[WRITE_BUFFER] = GENERIC_CMD,
+[READ_BUFFER] = GENERIC_CMD,
+[READ_LONG_10] = DISK_CMD|WORM_CMD|MOD_CMD,
+[WRITE_LONG_10] = DISK_CMD|WORM_CMD|MOD_CMD,
+[WRITE_SAME_10] = DISK_CMD,
+[UNMAP] = DISK_CMD,
+[READ_TOC] = ROM_CMD,
+[REPORT_DENSITY_SUPPORT]= TAPE_CMD,
+[GET_CONFIGURATION] = ROM_CMD,
+[LOG_SELECT]= GENERIC_CMD,
+[LOG_SENSE] = GENERIC_CMD,
+[MODE_SELECT_10]= GENERIC_CMD,
+[RESERVE_10]= PRINTER_CMD,
+[RELEASE_10]= PRINTER_CMD,
+[MODE_SENSE_10] = GENERIC_CMD,
+[PERSISTENT_RESERVE_IN] = GENERIC_CMD,
+[PERSISTENT_RESERVE_OUT]= GENERIC_CMD,
+[VARLENGTH_CDB] = OSD_CMD,
+[WRITE_FILEMARKS_16]= TAPE_CMD,
+[ATA_PASSTHROUGH] = DISK_CMD|ROM_CMD|RBC_CMD,
+[READ_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[WRITE_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[WRITE_VERIFY_16] = DISK_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[SYNCHRONIZE_CACHE_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[LOCATE_16] = TAPE_CMD,
+[WRITE_SAME_16] = DISK_CMD|TAPE_CMD,
+[SERVICE_ACTION_IN] = GENERIC_CMD,
+[REPORT_LUNS] = NO_ROM_CMD,
+[BLANK] = ROM_CMD,
+[MAINTENANCE_IN]= NO_ROM_CMD,
+[MAINTENANCE_OUT] = NO_ROM_CMD,
+[MOVE_MEDIUM] = MEDIUM_CHANGER_CMD,
+[LOAD_UNLOAD] = ROM_CMD|MEDIUM_CHANGER_CMD,
+[READ_12] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_12] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_VERIFY_12] = DISK_CMD|WORM_CMD|MOD_CMD,
+[VERIFY_12] = DISK_CMD|WORM_CMD|MOD_CMD
[PATCH 3/3] scsi-disk: Check for supported commands
Not every command is support for any device type. This patch adds
a check for rejecting unsupported commands.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/scsi-disk.c | 102
1 files changed, 0 insertions(+), 102 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 8ad90c0..6e985b4 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -361,100 +361,6 @@ static int scsi_get_sense(SCSIRequest *req, uint8_t
*outbuf, int len)
return scsi_build_sense(s-sense, outbuf, len, len 14);
}
-#define GENERIC_CMD (uint32_t)0x
-#define DISK_CMD (1u TYPE_DISK)
-#define TAPE_CMD (1u TYPE_TAPE)
-#define PRINTER_CMD (1u TYPE_PRINTER)
-#define PROCESSOR_CMD (1u TYPE_PROCESSOR)
-#define WORM_CMD (1u TYPE_WORM)
-#define ROM_CMD (1u TYPE_ROM)
-#define SCANNER_CMD (1u TYPE_SCANNER)
-#define MOD_CMD (1u TYPE_MOD)
-#define MEDIUM_CHANGER_CMD (1u TYPE_MEDIUM_CHANGER)
-#define ARRAY_CMD (1u TYPE_STORAGE_ARRAY)
-#define ENCLOSURE_CMD (1u TYPE_ENCLOSURE)
-#define RBC_CMD (1u TYPE_RBC)
-#define OSD_CMD (1u TYPE_OSD)
-
-#define NO_ROM_CMD (GENERIC_CMD | ~ROM_CMD)
-
-uint32_t scsi_cmd_table[0x100] = {
-[TEST_UNIT_READY] = GENERIC_CMD,
-[REWIND]= TAPE_CMD,
-[REQUEST_SENSE] = GENERIC_CMD,
-[FORMAT_UNIT] = DISK_CMD|ROM_CMD,
-[READ_BLOCK_LIMITS] = TAPE_CMD,
-[REASSIGN_BLOCKS] = DISK_CMD|WORM_CMD|MOD_CMD,
-[READ_6]= DISK_CMD|TAPE_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
-[WRITE_6] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD,
-[READ_REVERSE] = TAPE_CMD,
-[WRITE_FILEMARKS] = TAPE_CMD,
-[SPACE] = TAPE_CMD,
-[INQUIRY] = GENERIC_CMD,
-[MODE_SELECT] = GENERIC_CMD,
-[RESERVE] = TAPE_CMD|PRINTER_CMD,
-[RELEASE] = TAPE_CMD|PRINTER_CMD,
-[ERASE] = TAPE_CMD,
-[MODE_SENSE]= GENERIC_CMD,
-[START_STOP]= GENERIC_CMD,
-[RECEIVE_DIAGNOSTIC]= GENERIC_CMD,
-[SEND_DIAGNOSTIC] = GENERIC_CMD,
-[ALLOW_MEDIUM_REMOVAL] = GENERIC_CMD,
-[READ_CAPACITY_10] = DISK_CMD|WORM_CMD|MOD_CMD,
-[READ_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
-[WRITE_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
-[SEEK_10] = TAPE_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
-[WRITE_VERIFY_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
-[VERIFY_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
-[READ_POSITION] = TAPE_CMD,
-[SYNCHRONIZE_CACHE] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD|RBC_CMD,
-[WRITE_BUFFER] = GENERIC_CMD,
-[READ_BUFFER] = GENERIC_CMD,
-[READ_LONG_10] = DISK_CMD|WORM_CMD|MOD_CMD,
-[WRITE_LONG_10] = DISK_CMD|WORM_CMD|MOD_CMD,
-[WRITE_SAME_10] = DISK_CMD,
-[UNMAP] = DISK_CMD,
-[READ_TOC] = ROM_CMD,
-[REPORT_DENSITY_SUPPORT]= TAPE_CMD,
-[GET_CONFIGURATION] = ROM_CMD,
-[LOG_SELECT]= GENERIC_CMD,
-[LOG_SENSE] = GENERIC_CMD,
-[MODE_SELECT_10]= GENERIC_CMD,
-[RESERVE_10]= PRINTER_CMD,
-[RELEASE_10]= PRINTER_CMD,
-[MODE_SENSE_10] = GENERIC_CMD,
-[PERSISTENT_RESERVE_IN] = GENERIC_CMD,
-[PERSISTENT_RESERVE_OUT]= GENERIC_CMD,
-[VARLENGTH_CDB] = OSD_CMD,
-[WRITE_FILEMARKS_16]= TAPE_CMD,
-[ATA_PASSTHROUGH] = DISK_CMD|ROM_CMD|RBC_CMD,
-[READ_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
-[WRITE_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
-[WRITE_VERIFY_16] = DISK_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
-[SYNCHRONIZE_CACHE_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
-[LOCATE_16] = TAPE_CMD,
-[WRITE_SAME_16] = DISK_CMD|TAPE_CMD,
-[SERVICE_ACTION_IN] = GENERIC_CMD,
-[REPORT_LUNS] = NO_ROM_CMD,
-[BLANK] = ROM_CMD,
-[MAINTENANCE_IN]= NO_ROM_CMD,
-[MAINTENANCE_OUT] = NO_ROM_CMD,
-[MOVE_MEDIUM] = MEDIUM_CHANGER_CMD,
-[LOAD_UNLOAD] = ROM_CMD|MEDIUM_CHANGER_CMD,
-[READ_12] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
-[WRITE_12] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
-[WRITE_VERIFY_12] = DISK_CMD|WORM_CMD|MOD_CMD,
-[VERIFY_12] = DISK_CMD|WORM_CMD|MOD_CMD,
-[READ_ELEMENT_STATUS] = WORM_CMD|MOD_CMD,
-[SET_CD_SPEED] = ROM_CMD
-};
-
-static bool scsi_command_supported(uint8_t scsi_type, uint8_t cmd)
-{
-uint32_t mask
Re: [Qemu-devel] [PATCH 1/3] scsi: Sanitize command definitions
;
-}
Functional change, the commit message lies. Separate patch?
Matching update to scsi_cmd_table[] missing.
Looks like we could purge REZERO_UNIT elsewhere, too.
+case VERIFY_10:
break;
default:
scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
@@ -1052,14 +1046,13 @@ static int32_t scsi_send_command(SCSIRequest *req,
uint8_t *buf)
case RELEASE_10:
case START_STOP:
case ALLOW_MEDIUM_REMOVAL:
-case READ_CAPACITY:
+case READ_CAPACITY_10:
case SYNCHRONIZE_CACHE:
case READ_TOC:
case GET_CONFIGURATION:
case SERVICE_ACTION_IN:
case REPORT_LUNS:
-case VERIFY:
-case REZERO_UNIT:
+case VERIFY_10:
rc = scsi_disk_emulate_command(r, outbuf);
if (rc 0) {
return 0;
@@ -1082,7 +1075,7 @@ static int32_t scsi_send_command(SCSIRequest *req,
uint8_t *buf)
case WRITE_10:
case WRITE_12:
case WRITE_16:
-case WRITE_VERIFY:
+case WRITE_VERIFY_10:
case WRITE_VERIFY_12:
case WRITE_VERIFY_16:
len = r-req.cmd.xfer / s-qdev.blocksize;
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 90345a7..17e83c7 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -406,7 +406,7 @@ static int get_blocksize(BlockDriverState *bdrv)
memset(cmd, 0, sizeof(cmd));
memset(buf, 0, sizeof(buf));
-cmd[0] = READ_CAPACITY;
+cmd[0] = READ_CAPACITY_10;
memset(io_header, 0, sizeof(io_header));
io_header.interface_id = 'S';
OK, convinced. Will be doing a separate patch for removing the
obsolete commands.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/3] scsi-disk: Check for supported commands
On 07/22/2011 04:09 PM, Markus Armbruster wrote: Hannes Reineckeh...@suse.de writes: Not every command is support for any device type. This patch adds a check for rejecting unsupported commands. Signed-off-by: Hannes Reineckeh...@suse.de Commit message says patch adds, patch only deletes. Looks like something wrent wrong with 2/3 and 3/3. Argl. Yes, correct. Will be sending an updated patchset. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6][v2] Check for supported SCSI commands
Markus Armbruster pointed out that not every SCSI command is supported for a given device type. Based on his patch and suggestiongs this series cleans up the SCSI device type and adds a check for supported commands. Hannes Reinecke (6): scsi-disk: Codingstyle fixes scsi: Remove references to SET_WINDOW scsi: Remove REZERO_UNIT emulation scsi: Sanitize command definitions scsi-disk: Remove 'drive_kind' scsi-disk: Check for supported commands hw/scsi-bus.c | 74 - hw/scsi-defs.h| 62 +++--- hw/scsi-disk.c| 185 - hw/scsi-generic.c |2 +- 4 files changed, 221 insertions(+), 102 deletions(-) -- 1.7.3.4 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] scsi-disk: Codingstyle fixes
Replace tabs with spaces.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/scsi-disk.c | 16
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 05d14ab..910d3b5 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -526,7 +526,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
uint8_t *outbuf)
memset(outbuf, 0, buflen);
if (req-lun) {
-outbuf[0] = 0x7f; /* LUN not supported */
+outbuf[0] = 0x7f; /* LUN not supported */
return buflen;
}
@@ -836,7 +836,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r,
uint8_t *outbuf)
case TEST_UNIT_READY:
if (!bdrv_is_inserted(s-bs))
goto not_ready;
- break;
+break;
case REQUEST_SENSE:
if (req-cmd.xfer 4)
goto illegal_request;
@@ -848,7 +848,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r,
uint8_t *outbuf)
buflen = scsi_disk_emulate_inquiry(req, outbuf);
if (buflen 0)
goto illegal_request;
- break;
+break;
case MODE_SENSE:
case MODE_SENSE_10:
buflen = scsi_disk_emulate_mode_sense(req, outbuf);
@@ -881,14 +881,14 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r,
uint8_t *outbuf)
/* load/eject medium */
bdrv_eject(s-bs, !(req-cmd.buf[4] 1));
}
- break;
+break;
case ALLOW_MEDIUM_REMOVAL:
bdrv_set_locked(s-bs, req-cmd.buf[4] 1);
- break;
+break;
case READ_CAPACITY:
/* The normal LEN field for this command is zero. */
- memset(outbuf, 0, 8);
- bdrv_get_geometry(s-bs, nb_sectors);
+memset(outbuf, 0, 8);
+bdrv_get_geometry(s-bs, nb_sectors);
if (!nb_sectors)
goto not_ready;
nb_sectors /= s-cluster_size;
@@ -908,7 +908,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r,
uint8_t *outbuf)
outbuf[6] = s-cluster_size * 2;
outbuf[7] = 0;
buflen = 8;
- break;
+break;
case SYNCHRONIZE_CACHE:
ret = bdrv_flush(s-bs);
if (ret 0) {
--
1.7.3.4
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] scsi: Remove references to SET_WINDOW
SET_WINDOW command is vendor-specific only. So we shouldn't try to emulate it. Signed-off-by: Hannes Reinecke h...@suse.de --- hw/scsi-bus.c |2 -- hw/scsi-defs.h |1 - 2 files changed, 0 insertions(+), 3 deletions(-) diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index 8b1a412..facc98d 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -350,7 +350,6 @@ static void scsi_req_xfer_mode(SCSIRequest *req) case SEARCH_HIGH_12: case SEARCH_EQUAL_12: case SEARCH_LOW_12: -case SET_WINDOW: case MEDIUM_SCAN: case SEND_VOLUME_TAG: case WRITE_LONG_2: @@ -544,7 +543,6 @@ static const char *scsi_command_name(uint8_t cmd) [ SEND_DIAGNOSTIC ] = SEND_DIAGNOSTIC, [ ALLOW_MEDIUM_REMOVAL ] = ALLOW_MEDIUM_REMOVAL, -[ SET_WINDOW ] = SET_WINDOW, [ READ_CAPACITY] = READ_CAPACITY, [ READ_10 ] = READ_10, [ WRITE_10 ] = WRITE_10, diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h index 413cce0..8513983 100644 --- a/hw/scsi-defs.h +++ b/hw/scsi-defs.h @@ -49,7 +49,6 @@ #define SEND_DIAGNOSTIC 0x1d #define ALLOW_MEDIUM_REMOVAL 0x1e -#define SET_WINDOW0x24 #define READ_CAPACITY 0x25 #define READ_10 0x28 #define WRITE_10 0x2a -- 1.7.3.4 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] scsi: Remove REZERO_UNIT emulation
REZERO_UNIT command is obsolete. Remove support for it.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/scsi-bus.c |3 ---
hw/scsi-defs.h |1 -
hw/scsi-disk.c |7 ---
3 files changed, 0 insertions(+), 11 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index facc98d..52a6784 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -223,7 +223,6 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
switch(cmd[0]) {
case TEST_UNIT_READY:
-case REZERO_UNIT:
case START_STOP:
case SEEK_6:
case WRITE_FILEMARKS:
@@ -516,8 +515,6 @@ static const char *scsi_command_name(uint8_t cmd)
{
static const char *names[] = {
[ TEST_UNIT_READY ] = TEST_UNIT_READY,
-[ REZERO_UNIT ] = REZERO_UNIT,
-/* REWIND and REZERO_UNIT use the same operation code */
[ REQUEST_SENSE] = REQUEST_SENSE,
[ FORMAT_UNIT ] = FORMAT_UNIT,
[ READ_BLOCK_LIMITS] = READ_BLOCK_LIMITS,
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index 8513983..1f40c5c 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -25,7 +25,6 @@
*/
#define TEST_UNIT_READY 0x00
-#define REZERO_UNIT 0x01
#define REQUEST_SENSE 0x03
#define FORMAT_UNIT 0x04
#define READ_BLOCK_LIMITS 0x05
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 910d3b5..323368a 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -972,12 +972,6 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r,
uint8_t *outbuf)
break;
case VERIFY:
break;
-case REZERO_UNIT:
-DPRINTF(Rezero Unit\n);
-if (!bdrv_is_inserted(s-bs)) {
-goto not_ready;
-}
-break;
default:
scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
return -1;
@@ -1059,7 +1053,6 @@ static int32_t scsi_send_command(SCSIRequest *req,
uint8_t *buf)
case SERVICE_ACTION_IN:
case REPORT_LUNS:
case VERIFY:
-case REZERO_UNIT:
rc = scsi_disk_emulate_command(r, outbuf);
if (rc 0) {
return 0;
--
1.7.3.4
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] scsi-disk: Remove 'drive_kind'
Instead of using its own definitions scsi-disk should
be using the device type of the parent device.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/scsi-defs.h |6 +-
hw/scsi-disk.c | 48
2 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index f644860..27010b7 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -164,6 +164,7 @@
#define TYPE_DISK 0x00
#define TYPE_TAPE 0x01
+#define TYPE_PRINTER0x02
#define TYPE_PROCESSOR 0x03/* HP scanners use this */
#define TYPE_WORM 0x04/* Treated as ROM by our system */
#define TYPE_ROM0x05
@@ -171,6 +172,9 @@
#define TYPE_MOD0x07/* Magneto-optical disk -
* - treated as TYPE_DISK */
#define TYPE_MEDIUM_CHANGER 0x08
-#define TYPE_ENCLOSURE 0x0d/* Enclosure Services Device */
+#define TYPE_STORAGE_ARRAY 0x0c/* Storage array device */
+#define TYPE_ENCLOSURE 0x0d/* Enclosure Services Device */
+#define TYPE_RBC0x0e/* Simplified Direct-Access Device */
+#define TYPE_OSD0x11/* Object-storage Device */
#define TYPE_NO_LUN 0x7f
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 535fa11..ae2c157 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -74,7 +74,6 @@ struct SCSIDiskState
char *version;
char *serial;
SCSISense sense;
-SCSIDriveKind drive_kind;
};
static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
@@ -382,7 +381,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
uint8_t *outbuf)
return -1;
}
-if (s-drive_kind == SCSI_CD) {
+if (s-qdev.type == TYPE_ROM) {
outbuf[buflen++] = 5;
} else {
outbuf[buflen++] = 0;
@@ -401,7 +400,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
uint8_t *outbuf)
if (s-serial)
outbuf[buflen++] = 0x80; // unit serial number
outbuf[buflen++] = 0x83; // device identification
-if (s-drive_kind == SCSI_HD) {
+if (s-qdev.type == TYPE_DISK) {
outbuf[buflen++] = 0xb0; // block limits
outbuf[buflen++] = 0xb2; // thin provisioning
}
@@ -460,7 +459,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
uint8_t *outbuf)
unsigned int opt_io_size =
s-qdev.conf.opt_io_size / s-qdev.blocksize;
-if (s-drive_kind == SCSI_CD) {
+if (s-qdev.type == TYPE_ROM) {
DPRINTF(Inquiry (EVPD[%02X] not supported for CDROM\n,
page_code);
return -1;
@@ -530,12 +529,11 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
uint8_t *outbuf)
return buflen;
}
-if (s-drive_kind == SCSI_CD) {
-outbuf[0] = 5;
+outbuf[0] = s-qdev.type 0x1f;
+if (s-qdev.type == TYPE_ROM) {
outbuf[1] = 0x80;
memcpy(outbuf[16], QEMU CD-ROM , 16);
} else {
-outbuf[0] = 0;
outbuf[1] = s-removable ? 0x80 : 0;
memcpy(outbuf[16], QEMU HARDDISK , 16);
}
@@ -661,7 +659,7 @@ static int mode_sense_page(SCSIRequest *req, int page,
uint8_t *p,
return p[1] + 2;
case 0x2a: /* CD Capabilities and Mechanical Status page. */
-if (s-drive_kind != SCSI_CD)
+if (s-qdev.type != TYPE_ROM)
return 0;
p[0] = 0x2a;
p[1] = 0x14;
@@ -877,7 +875,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r,
uint8_t *outbuf)
goto illegal_request;
break;
case START_STOP:
-if (s-drive_kind == SCSI_CD (req-cmd.buf[4] 2)) {
+if (s-qdev.type == TYPE_ROM (req-cmd.buf[4] 2)) {
/* load/eject medium */
bdrv_eject(s-bs, !(req-cmd.buf[4] 1));
}
@@ -1183,7 +1181,7 @@ static void scsi_destroy(SCSIDevice *dev)
blockdev_mark_auto_del(s-qdev.conf.bs);
}
-static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind kind)
+static int scsi_initfn(SCSIDevice *dev, uint8_t scsi_type)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
DriveInfo *dinfo;
@@ -1193,9 +1191,8 @@ static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind
kind)
return -1;
}
s-bs = s-qdev.conf.bs;
-s-drive_kind = kind;
-if (kind == SCSI_HD !bdrv_is_inserted(s-bs)) {
+if (scsi_type == TYPE_DISK !bdrv_is_inserted(s-bs)) {
error_report(Device needs media, but drive is empty);
return -1;
}
@@ -1217,44 +1214,47 @@ static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind
kind)
return -1;
}
-if (kind == SCSI_CD) {
+if (scsi_type == TYPE_ROM) {
s-qdev.blocksize = 2048;
-} else {
+} else if (scsi_type == TYPE_DISK) {
s-qdev.blocksize = s
[PATCH 6/6] scsi-disk: Check for supported commands
Not every command is support for any device type. This patch adds
a check for rejecting unsupported commands.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/scsi-disk.c | 104 +++-
1 files changed, 103 insertions(+), 1 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index ae2c157..8ad90c0 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -361,13 +361,107 @@ static int scsi_get_sense(SCSIRequest *req, uint8_t
*outbuf, int len)
return scsi_build_sense(s-sense, outbuf, len, len 14);
}
+#define GENERIC_CMD (uint32_t)0x
+#define DISK_CMD (1u TYPE_DISK)
+#define TAPE_CMD (1u TYPE_TAPE)
+#define PRINTER_CMD (1u TYPE_PRINTER)
+#define PROCESSOR_CMD (1u TYPE_PROCESSOR)
+#define WORM_CMD (1u TYPE_WORM)
+#define ROM_CMD (1u TYPE_ROM)
+#define SCANNER_CMD (1u TYPE_SCANNER)
+#define MOD_CMD (1u TYPE_MOD)
+#define MEDIUM_CHANGER_CMD (1u TYPE_MEDIUM_CHANGER)
+#define ARRAY_CMD (1u TYPE_STORAGE_ARRAY)
+#define ENCLOSURE_CMD (1u TYPE_ENCLOSURE)
+#define RBC_CMD (1u TYPE_RBC)
+#define OSD_CMD (1u TYPE_OSD)
+
+#define NO_ROM_CMD (GENERIC_CMD | ~ROM_CMD)
+
+uint32_t scsi_cmd_table[0x100] = {
+[TEST_UNIT_READY] = GENERIC_CMD,
+[REWIND]= TAPE_CMD,
+[REQUEST_SENSE] = GENERIC_CMD,
+[FORMAT_UNIT] = DISK_CMD|ROM_CMD,
+[READ_BLOCK_LIMITS] = TAPE_CMD,
+[REASSIGN_BLOCKS] = DISK_CMD|WORM_CMD|MOD_CMD,
+[READ_6]= DISK_CMD|TAPE_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_6] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD,
+[READ_REVERSE] = TAPE_CMD,
+[WRITE_FILEMARKS] = TAPE_CMD,
+[SPACE] = TAPE_CMD,
+[INQUIRY] = GENERIC_CMD,
+[MODE_SELECT] = GENERIC_CMD,
+[RESERVE] = TAPE_CMD|PRINTER_CMD,
+[RELEASE] = TAPE_CMD|PRINTER_CMD,
+[ERASE] = TAPE_CMD,
+[MODE_SENSE]= GENERIC_CMD,
+[START_STOP]= GENERIC_CMD,
+[RECEIVE_DIAGNOSTIC]= GENERIC_CMD,
+[SEND_DIAGNOSTIC] = GENERIC_CMD,
+[ALLOW_MEDIUM_REMOVAL] = GENERIC_CMD,
+[READ_CAPACITY_10] = DISK_CMD|WORM_CMD|MOD_CMD,
+[READ_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[SEEK_10] = TAPE_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_VERIFY_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[VERIFY_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[READ_POSITION] = TAPE_CMD,
+[SYNCHRONIZE_CACHE] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD|RBC_CMD,
+[WRITE_BUFFER] = GENERIC_CMD,
+[READ_BUFFER] = GENERIC_CMD,
+[READ_LONG_10] = DISK_CMD|WORM_CMD|MOD_CMD,
+[WRITE_LONG_10] = DISK_CMD|WORM_CMD|MOD_CMD,
+[WRITE_SAME_10] = DISK_CMD,
+[UNMAP] = DISK_CMD,
+[READ_TOC] = ROM_CMD,
+[REPORT_DENSITY_SUPPORT]= TAPE_CMD,
+[GET_CONFIGURATION] = ROM_CMD,
+[LOG_SELECT]= GENERIC_CMD,
+[LOG_SENSE] = GENERIC_CMD,
+[MODE_SELECT_10]= GENERIC_CMD,
+[RESERVE_10]= PRINTER_CMD,
+[RELEASE_10]= PRINTER_CMD,
+[MODE_SENSE_10] = GENERIC_CMD,
+[PERSISTENT_RESERVE_IN] = GENERIC_CMD,
+[PERSISTENT_RESERVE_OUT]= GENERIC_CMD,
+[VARLENGTH_CDB] = OSD_CMD,
+[WRITE_FILEMARKS_16]= TAPE_CMD,
+[ATA_PASSTHROUGH] = DISK_CMD|ROM_CMD|RBC_CMD,
+[READ_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[WRITE_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[WRITE_VERIFY_16] = DISK_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[SYNCHRONIZE_CACHE_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[LOCATE_16] = TAPE_CMD,
+[WRITE_SAME_16] = DISK_CMD|TAPE_CMD,
+[SERVICE_ACTION_IN] = GENERIC_CMD,
+[REPORT_LUNS] = NO_ROM_CMD,
+[BLANK] = ROM_CMD,
+[MAINTENANCE_IN]= NO_ROM_CMD,
+[MAINTENANCE_OUT] = NO_ROM_CMD,
+[MOVE_MEDIUM] = MEDIUM_CHANGER_CMD,
+[LOAD_UNLOAD] = ROM_CMD|MEDIUM_CHANGER_CMD,
+[READ_12] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_12] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_VERIFY_12] = DISK_CMD|WORM_CMD|MOD_CMD,
+[VERIFY_12] = DISK_CMD|WORM_CMD|MOD_CMD,
+[READ_ELEMENT_STATUS] = WORM_CMD|MOD_CMD,
+[SET_CD_SPEED] = ROM_CMD
+};
+
+static bool scsi_command_supported(uint8_t scsi_type, uint8_t cmd)
+{
+uint32_t mask
[PATCH 4/6] scsi: Sanitize command definitions
Sanitize SCSI command definitions.
Add _10 suffix to READ_CAPACITY, WRITE_VERIFY, VERIFY, READ_LONG,
WRITE_LONG, and WRITE_SAME.
Add new command definitions for LOCATE_10, UNMAP, VARLENGTH_CDB,
WRITE_FILEMARKS_16, EXTENDED_COPY, ATA_PASSTHROUGH, ACCESS_CONTROL_IN,
ACCESS_CONTROL_OUT, COMPARE_AND_WRITE, VERIFY_16, SYNCHRONIZE_CACHE_16,
LOCATE_16, ERASE_16, WRITE_LONG_16, LOAD_UNLOAD, VERIFY_12.
Remove invalid definition of WRITE_LONG_2.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/scsi-bus.c | 69
hw/scsi-defs.h| 54 +
hw/scsi-disk.c| 10
hw/scsi-generic.c |2 +-
4 files changed, 81 insertions(+), 54 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 52a6784..0b0344c 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -223,6 +223,7 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
switch(cmd[0]) {
case TEST_UNIT_READY:
+case REWIND:
case START_STOP:
case SEEK_6:
case WRITE_FILEMARKS:
@@ -231,24 +232,24 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
case RELEASE:
case ERASE:
case ALLOW_MEDIUM_REMOVAL:
-case VERIFY:
+case VERIFY_10:
case SEEK_10:
case SYNCHRONIZE_CACHE:
case LOCK_UNLOCK_CACHE:
case LOAD_UNLOAD:
case SET_CD_SPEED:
case SET_LIMITS:
-case WRITE_LONG:
+case WRITE_LONG_10:
case MOVE_MEDIUM:
case UPDATE_BLOCK:
req-cmd.xfer = 0;
break;
case MODE_SENSE:
break;
-case WRITE_SAME:
+case WRITE_SAME_10:
req-cmd.xfer = 1;
break;
-case READ_CAPACITY:
+case READ_CAPACITY_10:
req-cmd.xfer = 8;
break;
case READ_BLOCK_LIMITS:
@@ -264,7 +265,7 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
req-cmd.xfer *= 8;
break;
case WRITE_10:
-case WRITE_VERIFY:
+case WRITE_VERIFY_10:
case WRITE_6:
case WRITE_12:
case WRITE_VERIFY_12:
@@ -324,7 +325,7 @@ static void scsi_req_xfer_mode(SCSIRequest *req)
switch (req-cmd.buf[0]) {
case WRITE_6:
case WRITE_10:
-case WRITE_VERIFY:
+case WRITE_VERIFY_10:
case WRITE_12:
case WRITE_VERIFY_12:
case WRITE_16:
@@ -344,14 +345,13 @@ static void scsi_req_xfer_mode(SCSIRequest *req)
case SEARCH_HIGH:
case SEARCH_LOW:
case UPDATE_BLOCK:
-case WRITE_LONG:
-case WRITE_SAME:
+case WRITE_LONG_10:
+case WRITE_SAME_10:
case SEARCH_HIGH_12:
case SEARCH_EQUAL_12:
case SEARCH_LOW_12:
case MEDIUM_SCAN:
case SEND_VOLUME_TAG:
-case WRITE_LONG_2:
case PERSISTENT_RESERVE_OUT:
case MAINTENANCE_OUT:
req-cmd.mode = SCSI_XFER_TO_DEV;
@@ -515,6 +515,7 @@ static const char *scsi_command_name(uint8_t cmd)
{
static const char *names[] = {
[ TEST_UNIT_READY ] = TEST_UNIT_READY,
+[ REWIND ] = REWIND,
[ REQUEST_SENSE] = REQUEST_SENSE,
[ FORMAT_UNIT ] = FORMAT_UNIT,
[ READ_BLOCK_LIMITS] = READ_BLOCK_LIMITS,
@@ -539,13 +540,12 @@ static const char *scsi_command_name(uint8_t cmd)
[ RECEIVE_DIAGNOSTIC ] = RECEIVE_DIAGNOSTIC,
[ SEND_DIAGNOSTIC ] = SEND_DIAGNOSTIC,
[ ALLOW_MEDIUM_REMOVAL ] = ALLOW_MEDIUM_REMOVAL,
-
-[ READ_CAPACITY] = READ_CAPACITY,
+[ READ_CAPACITY_10 ] = READ_CAPACITY_10,
[ READ_10 ] = READ_10,
[ WRITE_10 ] = WRITE_10,
[ SEEK_10 ] = SEEK_10,
-[ WRITE_VERIFY ] = WRITE_VERIFY,
-[ VERIFY ] = VERIFY,
+[ WRITE_VERIFY_10 ] = WRITE_VERIFY_10,
+[ VERIFY_10] = VERIFY_10,
[ SEARCH_HIGH ] = SEARCH_HIGH,
[ SEARCH_EQUAL ] = SEARCH_EQUAL,
[ SEARCH_LOW ] = SEARCH_LOW,
@@ -561,11 +561,14 @@ static const char *scsi_command_name(uint8_t cmd)
[ WRITE_BUFFER ] = WRITE_BUFFER,
[ READ_BUFFER ] = READ_BUFFER,
[ UPDATE_BLOCK ] = UPDATE_BLOCK,
-[ READ_LONG] = READ_LONG,
-[ WRITE_LONG ] = WRITE_LONG,
+[ READ_LONG_10 ] = READ_LONG_10,
+[ WRITE_LONG_10] = WRITE_LONG_10,
[ CHANGE_DEFINITION] = CHANGE_DEFINITION,
-[ WRITE_SAME ] = WRITE_SAME,
+[ WRITE_SAME_10] = WRITE_SAME_10,
+[ UNMAP] = UNMAP,
[ READ_TOC ] = READ_TOC,
+[ REPORT_DENSITY_SUPPORT ] = REPORT_DENSITY_SUPPORT,
+[ GET_CONFIGURATION] = GET_CONFIGURATION,
[ LOG_SELECT ] = LOG_SELECT,
[ LOG_SENSE
Re: [Qemu-devel] [PATCH 0/6][v2] Check for supported SCSI commands
On 07/25/2011 06:04 PM, Markus Armbruster wrote: Hannes Reineckeh...@suse.de writes: Markus Armbruster pointed out that not every SCSI command is supported for a given device type. Based on his patch and suggestiongs this series cleans up the SCSI device type and adds a check for supported commands. I like this series. It conflicts with mine. I can work with Kevin to resolve the conflict. Yes please. I'm happy to assist. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 5/6] scsi-disk: Remove 'drive_kind'
On 07/25/2011 05:59 PM, Markus Armbruster wrote:
Hannes Reineckeh...@suse.de writes:
Instead of using its own definitions scsi-disk should
be using the device type of the parent device.
Signed-off-by: Hannes Reineckeh...@suse.de
---
hw/scsi-defs.h |6 +-
hw/scsi-disk.c | 48
2 files changed, 29 insertions(+), 25 deletions(-)
[ .. ]
@@ -1217,44 +1214,47 @@ static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind
kind)
return -1;
}
-if (kind == SCSI_CD) {
+if (scsi_type == TYPE_ROM) {
s-qdev.blocksize = 2048;
-} else {
+} else if (scsi_type == TYPE_DISK) {
s-qdev.blocksize = s-qdev.conf.logical_block_size;
+} else {
+error_report(scsi-disk: Unhandled SCSI type %02x, scsi_type);
+return -1;
}
s-cluster_size = s-qdev.blocksize / 512;
s-bs-buffer_alignment = s-qdev.blocksize;
-s-qdev.type = TYPE_DISK;
+s-qdev.type = scsi_type;
Is this a bug fix?
No, proper initialisation.
s-qdev.type is the SCSI type we're told to emulate. So we have to
set it to the correct value otherwise the emulation will return
wrong values.
qemu_add_vm_change_state_handler(scsi_dma_restart_cb, s);
-bdrv_set_removable(s-bs, kind == SCSI_CD);
+bdrv_set_removable(s-bs, scsi_type == TYPE_ROM);
add_boot_device_path(s-qdev.conf.bootindex,dev-qdev, ,0);
return 0;
}
static int scsi_hd_initfn(SCSIDevice *dev)
{
-return scsi_initfn(dev, SCSI_HD);
+return scsi_initfn(dev, TYPE_DISK);
}
static int scsi_cd_initfn(SCSIDevice *dev)
{
-return scsi_initfn(dev, SCSI_CD);
+return scsi_initfn(dev, TYPE_ROM);
}
static int scsi_disk_initfn(SCSIDevice *dev)
{
-SCSIDriveKind kind;
DriveInfo *dinfo;
+uint8_t scsi_type = TYPE_DISK;
-if (!dev-conf.bs) {
-kind = SCSI_HD; /* will die in scsi_initfn() */
The comment explains why we don't explicitly fail when !dev-conf.bs,
like all the other block device models. I'd rather keep it.
Ah. The magic of block devices. By all means, keep it.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 5/6] scsi-disk: Remove 'drive_kind'
On 07/26/2011 08:38 AM, Markus Armbruster wrote:
Hannes Reineckeh...@suse.de writes:
On 07/25/2011 05:59 PM, Markus Armbruster wrote:
Hannes Reineckeh...@suse.de writes:
Instead of using its own definitions scsi-disk should
be using the device type of the parent device.
Signed-off-by: Hannes Reineckeh...@suse.de
---
hw/scsi-defs.h |6 +-
hw/scsi-disk.c | 48
2 files changed, 29 insertions(+), 25 deletions(-)
[ .. ]
@@ -1217,44 +1214,47 @@ static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind
kind)
return -1;
}
-if (kind == SCSI_CD) {
+if (scsi_type == TYPE_ROM) {
s-qdev.blocksize = 2048;
-} else {
+} else if (scsi_type == TYPE_DISK) {
s-qdev.blocksize = s-qdev.conf.logical_block_size;
+} else {
+error_report(scsi-disk: Unhandled SCSI type %02x, scsi_type);
+return -1;
}
s-cluster_size = s-qdev.blocksize / 512;
s-bs-buffer_alignment = s-qdev.blocksize;
-s-qdev.type = TYPE_DISK;
+s-qdev.type = scsi_type;
Is this a bug fix?
No, proper initialisation.
s-qdev.type is the SCSI type we're told to emulate. So we have to set
it to the correct value otherwise the emulation will return wrong
values.
Well, it changes .type from TYPE_DISK to TYPE_ROM for scsi-cd. I
understand how that is required for your patch to work. I wonder
whether it has an impact beyond that, given that the old value is
plainly wrong.
qemu_add_vm_change_state_handler(scsi_dma_restart_cb, s);
-bdrv_set_removable(s-bs, kind == SCSI_CD);
+bdrv_set_removable(s-bs, scsi_type == TYPE_ROM);
add_boot_device_path(s-qdev.conf.bootindex,dev-qdev, ,0);
return 0;
}
static int scsi_hd_initfn(SCSIDevice *dev)
{
-return scsi_initfn(dev, SCSI_HD);
+return scsi_initfn(dev, TYPE_DISK);
}
static int scsi_cd_initfn(SCSIDevice *dev)
{
-return scsi_initfn(dev, SCSI_CD);
+return scsi_initfn(dev, TYPE_ROM);
}
static int scsi_disk_initfn(SCSIDevice *dev)
{
-SCSIDriveKind kind;
DriveInfo *dinfo;
+uint8_t scsi_type = TYPE_DISK;
-if (!dev-conf.bs) {
-kind = SCSI_HD; /* will die in scsi_initfn() */
The comment explains why we don't explicitly fail when !dev-conf.bs,
like all the other block device models. I'd rather keep it.
Ah. The magic of block devices. By all means, keep it.
Like this?
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index f42a5d1..0925944 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1251,17 +1251,17 @@ static int scsi_cd_initfn(SCSIDevice *dev)
static int scsi_disk_initfn(SCSIDevice *dev)
{
-SCSIDriveKind kind;
+uint8_t scsi_type;
DriveInfo *dinfo;
if (!dev-conf.bs) {
-kind = SCSI_HD; /* will die in scsi_initfn() */
+scsi_type = TYPE_DISK; /* will die in scsi_initfn() */
} else {
dinfo = drive_get_by_blockdev(dev-conf.bs);
-kind = dinfo-media_cd ? SCSI_CD : SCSI_HD;
+scsi_type = dinfo-media_cd ? TYPE_ROM : TYPE_DISK;
}
-return scsi_initfn(dev, kind);
+return scsi_initfn(dev, scsi_type);
}
#define DEFINE_SCSI_DISK_PROPERTIES() \
By the way, I'm afraid you forgot to remove typedef SCSIDriveKind. Care
to respin this one?
Here you go.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
From f6e40a484dbcfdd4f8434ae3427c75a56581d659 Mon Sep 17 00:00:00 2001
From: Hannes Reinecke h...@suse.de
Date: Fri, 22 Jul 2011 16:44:46 +0200
Subject: [PATCH] scsi-disk: Remove 'drive_kind'
Instead of using its own definitions scsi-disk should
be using the device type of the parent device.
Signed-off-by: Hannes Reinecke h...@suse.de
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index f644860..27010b7 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -164,6 +164,7 @@
#define TYPE_DISK 0x00
#define TYPE_TAPE 0x01
+#define TYPE_PRINTER0x02
#define TYPE_PROCESSOR 0x03/* HP scanners use this */
#define TYPE_WORM 0x04/* Treated as ROM by our system */
#define TYPE_ROM0x05
@@ -171,6 +172,9 @@
#define TYPE_MOD0x07/* Magneto-optical disk -
* - treated as TYPE_DISK */
#define TYPE_MEDIUM_CHANGER 0x08
-#define TYPE_ENCLOSURE 0x0d/* Enclosure Services Device */
+#define TYPE_STORAGE_ARRAY 0x0c/* Storage array device */
+#define TYPE_ENCLOSURE 0x0d/* Enclosure Services Device */
+#define TYPE_RBC0x0e/* Simplified Direct-Access Device */
+#define TYPE_OSD0x11/* Object-storage Device */
#define TYPE_NO_LUN 0x7f
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 535fa11..f2511d0 100644
Re: [PATCH 6/6] scsi-disk: Check for supported commands
On 07/26/2011 03:07 PM, Kevin Wolf wrote:
Am 22.07.2011 16:51, schrieb Hannes Reinecke:
Not every command is support for any device type. This patch adds
a check for rejecting unsupported commands.
Signed-off-by: Hannes Reineckeh...@suse.de
---
hw/scsi-disk.c | 104 +++-
1 files changed, 103 insertions(+), 1 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index ae2c157..8ad90c0 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -361,13 +361,107 @@ static int scsi_get_sense(SCSIRequest *req, uint8_t
*outbuf, int len)
return scsi_build_sense(s-sense, outbuf, len, len 14);
}
+#define GENERIC_CMD (uint32_t)0x
+#define DISK_CMD (1u TYPE_DISK)
+#define TAPE_CMD (1u TYPE_TAPE)
+#define PRINTER_CMD (1u TYPE_PRINTER)
+#define PROCESSOR_CMD (1u TYPE_PROCESSOR)
+#define WORM_CMD (1u TYPE_WORM)
+#define ROM_CMD (1u TYPE_ROM)
+#define SCANNER_CMD (1u TYPE_SCANNER)
+#define MOD_CMD (1u TYPE_MOD)
+#define MEDIUM_CHANGER_CMD (1u TYPE_MEDIUM_CHANGER)
+#define ARRAY_CMD (1u TYPE_STORAGE_ARRAY)
+#define ENCLOSURE_CMD (1u TYPE_ENCLOSURE)
+#define RBC_CMD (1u TYPE_RBC)
+#define OSD_CMD (1u TYPE_OSD)
+
+#define NO_ROM_CMD (GENERIC_CMD | ~ROM_CMD)
+
+uint32_t scsi_cmd_table[0x100] = {
+[TEST_UNIT_READY] = GENERIC_CMD,
+[REWIND]= TAPE_CMD,
+[REQUEST_SENSE] = GENERIC_CMD,
+[FORMAT_UNIT] = DISK_CMD|ROM_CMD,
+[READ_BLOCK_LIMITS] = TAPE_CMD,
+[REASSIGN_BLOCKS] = DISK_CMD|WORM_CMD|MOD_CMD,
+[READ_6]= DISK_CMD|TAPE_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_6] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD,
+[READ_REVERSE] = TAPE_CMD,
+[WRITE_FILEMARKS] = TAPE_CMD,
+[SPACE] = TAPE_CMD,
+[INQUIRY] = GENERIC_CMD,
+[MODE_SELECT] = GENERIC_CMD,
+[RESERVE] = TAPE_CMD|PRINTER_CMD,
+[RELEASE] = TAPE_CMD|PRINTER_CMD,
+[ERASE] = TAPE_CMD,
+[MODE_SENSE]= GENERIC_CMD,
+[START_STOP]= GENERIC_CMD,
+[RECEIVE_DIAGNOSTIC]= GENERIC_CMD,
+[SEND_DIAGNOSTIC] = GENERIC_CMD,
+[ALLOW_MEDIUM_REMOVAL] = GENERIC_CMD,
+[READ_CAPACITY_10] = DISK_CMD|WORM_CMD|MOD_CMD,
+[READ_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[SEEK_10] = TAPE_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_VERIFY_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[VERIFY_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[READ_POSITION] = TAPE_CMD,
+[SYNCHRONIZE_CACHE] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD|RBC_CMD,
+[WRITE_BUFFER] = GENERIC_CMD,
+[READ_BUFFER] = GENERIC_CMD,
+[READ_LONG_10] = DISK_CMD|WORM_CMD|MOD_CMD,
+[WRITE_LONG_10] = DISK_CMD|WORM_CMD|MOD_CMD,
+[WRITE_SAME_10] = DISK_CMD,
+[UNMAP] = DISK_CMD,
+[READ_TOC] = ROM_CMD,
+[REPORT_DENSITY_SUPPORT]= TAPE_CMD,
+[GET_CONFIGURATION] = ROM_CMD,
+[LOG_SELECT]= GENERIC_CMD,
+[LOG_SENSE] = GENERIC_CMD,
+[MODE_SELECT_10]= GENERIC_CMD,
+[RESERVE_10]= PRINTER_CMD,
+[RELEASE_10]= PRINTER_CMD,
+[MODE_SENSE_10] = GENERIC_CMD,
+[PERSISTENT_RESERVE_IN] = GENERIC_CMD,
+[PERSISTENT_RESERVE_OUT]= GENERIC_CMD,
+[VARLENGTH_CDB] = OSD_CMD,
+[WRITE_FILEMARKS_16]= TAPE_CMD,
+[ATA_PASSTHROUGH] = DISK_CMD|ROM_CMD|RBC_CMD,
+[READ_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[WRITE_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[WRITE_VERIFY_16] = DISK_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[SYNCHRONIZE_CACHE_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[LOCATE_16] = TAPE_CMD,
+[WRITE_SAME_16] = DISK_CMD|TAPE_CMD,
+[SERVICE_ACTION_IN] = GENERIC_CMD,
+[REPORT_LUNS] = NO_ROM_CMD,
+[BLANK] = ROM_CMD,
+[MAINTENANCE_IN]= NO_ROM_CMD,
+[MAINTENANCE_OUT] = NO_ROM_CMD,
+[MOVE_MEDIUM] = MEDIUM_CHANGER_CMD,
+[LOAD_UNLOAD] = ROM_CMD|MEDIUM_CHANGER_CMD,
+[READ_12] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_12] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_VERIFY_12] = DISK_CMD|WORM_CMD|MOD_CMD,
+[VERIFY_12] = DISK_CMD|WORM_CMD|MOD_CMD,
+[READ_ELEMENT_STATUS] = WORM_CMD|MOD_CMD,
+[SET_CD_SPEED] = ROM_CMD
Re: [PATCH 6/6] scsi-disk: Check for supported commands
On 07/26/2011 03:46 PM, Kevin Wolf wrote:
Am 22.07.2011 16:51, schrieb Hannes Reinecke:
Not every command is support for any device type. This patch adds
a check for rejecting unsupported commands.
Signed-off-by: Hannes Reineckeh...@suse.de
We do emulate SEEK (6), but it's not in your scsi_cmd_table at all.
Hmm.
---
hw/scsi-disk.c | 104 +++-
1 files changed, 103 insertions(+), 1 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index ae2c157..8ad90c0 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -361,13 +361,107 @@ static int scsi_get_sense(SCSIRequest *req, uint8_t
*outbuf, int len)
return scsi_build_sense(s-sense, outbuf, len, len 14);
}
+#define GENERIC_CMD (uint32_t)0x
+#define DISK_CMD (1u TYPE_DISK)
+#define TAPE_CMD (1u TYPE_TAPE)
+#define PRINTER_CMD (1u TYPE_PRINTER)
+#define PROCESSOR_CMD (1u TYPE_PROCESSOR)
+#define WORM_CMD (1u TYPE_WORM)
+#define ROM_CMD (1u TYPE_ROM)
+#define SCANNER_CMD (1u TYPE_SCANNER)
+#define MOD_CMD (1u TYPE_MOD)
+#define MEDIUM_CHANGER_CMD (1u TYPE_MEDIUM_CHANGER)
+#define ARRAY_CMD (1u TYPE_STORAGE_ARRAY)
+#define ENCLOSURE_CMD (1u TYPE_ENCLOSURE)
+#define RBC_CMD (1u TYPE_RBC)
+#define OSD_CMD (1u TYPE_OSD)
+
+#define NO_ROM_CMD (GENERIC_CMD | ~ROM_CMD)
+
+uint32_t scsi_cmd_table[0x100] = {
+[TEST_UNIT_READY] = GENERIC_CMD,
+[REWIND]= TAPE_CMD,
+[REQUEST_SENSE] = GENERIC_CMD,
+[FORMAT_UNIT] = DISK_CMD|ROM_CMD,
+[READ_BLOCK_LIMITS] = TAPE_CMD,
+[REASSIGN_BLOCKS] = DISK_CMD|WORM_CMD|MOD_CMD,
+[READ_6]= DISK_CMD|TAPE_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
My spec says that MMC doesn't support READ(6)
But it does support 'RECEIVE(6)', with the same opcode.
+[WRITE_6] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD,
+[READ_REVERSE] = TAPE_CMD,
+[WRITE_FILEMARKS] = TAPE_CMD,
+[SPACE] = TAPE_CMD,
+[INQUIRY] = GENERIC_CMD,
+[MODE_SELECT] = GENERIC_CMD,
+[RESERVE] = TAPE_CMD|PRINTER_CMD,
+[RELEASE] = TAPE_CMD|PRINTER_CMD,
What's the reason for allowing this for tape, but not e.g. for disks?
It's marked as obsolete for both (and optional for quite a few other types)
Because it's mandatory for TAPE and PRINTER. But the implementation
details are horrible and we're not doing anything here (which in
itself is a violation of the spec), so I think it's better to
not support it if we don't have to.
+[ERASE] = TAPE_CMD,
+[MODE_SENSE]= GENERIC_CMD,
+[START_STOP]= GENERIC_CMD,
+[RECEIVE_DIAGNOSTIC]= GENERIC_CMD,
+[SEND_DIAGNOSTIC] = GENERIC_CMD,
+[ALLOW_MEDIUM_REMOVAL] = GENERIC_CMD,
+[READ_CAPACITY_10] = DISK_CMD|WORM_CMD|MOD_CMD,
ROM_CMD, too
Ok.
+[READ_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[WRITE_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[SEEK_10] = TAPE_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
Tape doesn't have SEEK(10) in the spec.
But it does have 'LOCATE_10', which shares the same opcode.
+[WRITE_VERIFY_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[VERIFY_10] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
+[READ_POSITION] = TAPE_CMD,
+[SYNCHRONIZE_CACHE] = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD|RBC_CMD,
+[WRITE_BUFFER] = GENERIC_CMD,
+[READ_BUFFER] = GENERIC_CMD,
+[READ_LONG_10] = DISK_CMD|WORM_CMD|MOD_CMD,
+[WRITE_LONG_10] = DISK_CMD|WORM_CMD|MOD_CMD,
+[WRITE_SAME_10] = DISK_CMD,
+[UNMAP] = DISK_CMD,
+[READ_TOC] = ROM_CMD,
+[REPORT_DENSITY_SUPPORT]= TAPE_CMD,
+[GET_CONFIGURATION] = ROM_CMD,
+[LOG_SELECT]= GENERIC_CMD,
+[LOG_SENSE] = GENERIC_CMD,
+[MODE_SELECT_10]= GENERIC_CMD,
+[RESERVE_10]= PRINTER_CMD,
+[RELEASE_10]= PRINTER_CMD,
+[MODE_SENSE_10] = GENERIC_CMD,
+[PERSISTENT_RESERVE_IN] = GENERIC_CMD,
+[PERSISTENT_RESERVE_OUT]= GENERIC_CMD,
+[VARLENGTH_CDB] = OSD_CMD,
+[WRITE_FILEMARKS_16]= TAPE_CMD,
+[ATA_PASSTHROUGH] = DISK_CMD|ROM_CMD|RBC_CMD,
+[READ_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[WRITE_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[WRITE_VERIFY_16] = DISK_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
+[SYNCHRONIZE_CACHE_16] = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
Tape doesn't have this.
It's called 'SPACE(16)' for tapes.
+[LOCATE_16] = TAPE_CMD
Re: Pe: [PATCH v5 1/3] virtio-scsi: first version
On 02/13/2012 02:18 PM, Michael S. Tsirkin wrote: On Tue, Feb 14, 2012 at 12:13:36AM +1100, ronnie sahlberg wrote: On Tue, Feb 14, 2012 at 12:00 AM, Michael S. Tsirkin m...@redhat.com wrote: On Mon, Feb 13, 2012 at 02:54:03PM +0200, Dor Laor wrote: Only if you use the pci multi-function option but that kills standard hot unplug It doesn't kill it as such, rather you can't unplug luns individually. Isnt that just a consequence of the current implementation rather than a SCSI limitation? Yes. A different way to do hoplug could be to flag all devices as removable in the standard inq page then leave the LUN there persistently and what you remove/add is not the LUN device itself but just the media in the device. Instead of hot-plug remove the LUN, hot-plug becomes media eject or media insert. The device remains present all time, you never remove it, but instead hot-plug controls if the media is present or not. This would require implementing at least START_STOP_UNIT and PREVENT_ALLOW_MEDIUM_REMOVAL opcode emulation from SBC. regards ronnie sahlberg That would work. Or we simply use the Peripheral Qualifier that the device is gone; eg we could simply set PQ = 1, return sense code 0x25/00 and be done with ... Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: about NPIV with qemu-kvm.
On 10/26/2011 06:40 AM, ya su wrote: hi, hannes: I want to use NPIV with qemu-kvm, I issued the following command: echo ':' /sys/class/fc_host/host0/vport_create and it will produce a new host6 and one vport succesfully, but it does not create any virtual hba pci device. so I don't know how to assign the virtual host to qemu-kvm. Well, you can't. There is no mechanism for. When using NPIV you need to pass in the individual LUNs via eg virtio-blk. from your this mail, does array will first need to assign a lun to this vport? and through this new created disk, like device /dev/sdf, then I add qemu-kvm with -drive file=/dev/sdf,if=virtio... arguments? Yes. That's what you need to do. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio scsi host draft specification, v3
[]; u8 sense[sense_size]; u32 sense_len; u32 residual; u16 status_qualifier; u8 status; u8 response; char datain[]; }; /* command-specific response values */ #define VIRTIO_SCSI_S_OK 0 #define VIRTIO_SCSI_S_UNDERRUN1 #define VIRTIO_SCSI_S_ABORTED 2 #define VIRTIO_SCSI_S_FAILURE 3 /* task_attr */ #define VIRTIO_SCSI_S_SIMPLE 0 #define VIRTIO_SCSI_S_ORDERED 1 #define VIRTIO_SCSI_S_HEAD2 #define VIRTIO_SCSI_S_ACA 3 The lun field addresses a bus, target and logical unit in the SCSI host. The id field is the command identifier as defined in SAM. Please do not rely in bus/target/lun here. These are leftovers from parallel SCSI and do not have any meaning on modern SCSI implementation (eg FC or SAS). Rephrase that to The lun field is the Logical Unit Number as defined in SAM. Task_attr, prio and CRN are defined in SAM. The prio field should always be zero, as command priority is explicitly not supported by this version of the device. task_attr defines the task attribute as in the table above, Note that all task attributes may be mapped to SIMPLE by the device. CRN is generally expected to be 0, but clients can provide it. The maximum CRN value defined by the protocol is 255, since CRN is stored in an 8-bit integer. All of these fields are always read-only, as are the cdb and dataout field. sense and subsequent fields are always write-only. The sense_len field indicates the number of bytes actually written to the sense buffer. The residual field indicates the residual size, calculated as data_length - number_of_transferred_bytes, for read or write operations. The status byte is written by the device to be the SCSI status code. ?? I doubt that exists. Make that: The status byte is written by the device to be the status code as defined in SAM. The response byte is written by the device to be one of the following: - VIRTIO_SCSI_S_OK when the request was completed and the status byte is filled with a SCSI status code (not necessarily GOOD). - VIRTIO_SCSI_S_UNDERRUN if the content of the CDB requires transferring more data than is available in the data buffers. - VIRTIO_SCSI_S_ABORTED if the request was cancelled due to a reset or another task management function. - VIRTIO_SCSI_S_FAILURE for other host or guest error. In particular, if neither dataout nor datain is empty, and the VIRTIO_SCSI_F_INOUT feature has not been negotiated, the request will be immediately returned with a response equal to VIRTIO_SCSI_S_FAILURE. And, of course: VIRTIO_SCSI_S_DISCONNECT if the request could not be processed due to a communication failure (eg device was removed or could not be reached). The remaining bits seem to be okay. One general question: This specification implies a strict one-to-one mapping between host and target. IE there is no way of specifying more than one target per host. This will make things like ALUA (Asymmetric Logical Unit Access) a bit tricky to implement, as the port states there are bound to target port groups. So with the virtio host spec we would need to specify two hosts to represent that. If that's the intention here I'm fine, but maybe we should be specifying this expressis verbis somewhere. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio scsi host draft specification, v3
On 06/10/2011 04:35 PM, Paolo Bonzini wrote: If requests are placed on arbitrary queues you'll inevitably run on locking issues to ensure strict request ordering. I would add here: If a device uses more than one queue it is the responsibility of the device to ensure strict request ordering. Applied with s/device/guest/g. Please do not rely in bus/target/lun here. These are leftovers from parallel SCSI and do not have any meaning on modern SCSI implementation (eg FC or SAS). Rephrase that to The lun field is the Logical Unit Number as defined in SAM. Ok. The status byte is written by the device to be the SCSI status code. ?? I doubt that exists. Make that: The status byte is written by the device to be the status code as defined in SAM. Ok. The response byte is written by the device to be one of the following: - VIRTIO_SCSI_S_OK when the request was completed and the status byte is filled with a SCSI status code (not necessarily GOOD). - VIRTIO_SCSI_S_UNDERRUN if the content of the CDB requires transferring more data than is available in the data buffers. - VIRTIO_SCSI_S_ABORTED if the request was cancelled due to a reset or another task management function. - VIRTIO_SCSI_S_FAILURE for other host or guest error. In particular, if neither dataout nor datain is empty, and the VIRTIO_SCSI_F_INOUT feature has not been negotiated, the request will be immediately returned with a response equal to VIRTIO_SCSI_S_FAILURE. And, of course: VIRTIO_SCSI_S_DISCONNECT if the request could not be processed due to a communication failure (eg device was removed or could not be reached). Ok. This specification implies a strict one-to-one mapping between host and target. IE there is no way of specifying more than one target per host. Actually no, the intention is to use hierarchical LUNs to support more than one target per host. Can't. Hierarchical LUNs is a target-internal representation. The initiator (ie guest OS) should _not_ try to assume anything about the internal structure and just use the LUN as an opaque number. Reason being that the LUN addressing is not unique, and there are several choices on how to represent a given LUN. So the consensus here is that different LUN numbers represent different physical devices, regardless on the (internal) LUN representation. Which in turn means we cannot use the LUN number to convey anything else than a device identification relative to a target. Cf SAM-3 paragraph 4.8: A logical unit number is a field (see 4.9) containing 64 bits that identifies the logical unit within a SCSI target device when accessed by a SCSI target port. IE the LUN is dependent on the target, but you cannot make assumptions on the target. Consequently, it's in the hosts' responsibility to figure out the targets in the system. After that it invokes the 'scan' function from the SCSI midlayer. You can't start from a LUN and try to figure out the targets ... If you want to support more than on target per host you need some sort of enumeration/callback which allows the host to figure out the number of available targets. But in general the targets are referenced by the target port identifier as specified in the appropriate standard (eg FC or SAS). Sadly, we don't have any standard to fall back on for this. If, however, we decide to expose some details about the backend, we could be using the values from the backend directly. EG we could be forwarding the SCSI target port identifier here (if backed by real hardware) or creating our own SAS-type identifier when backed by qemu block. Then we could just query the backend via a new command on the controlq (eg 'list target ports') and wouldn't have to worry about any protocol specific details here. Of course, when doing so we would be lose the ability to freely remap LUNs. But then remapping LUNs doesn't gain you much imho. Plus you could always use qemu block backend here if you want to hide the details. But we would be finally able to use NPIV for KVM, something I wanted to do for a _long_ time. I personally _really_ would like to see the real backing device details exposed to the guest. Otherwise the more advanced stuff like persistent reservations becomes _really_ hard if not impossible. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio scsi host draft specification, v3
On 06/12/2011 09:51 AM, Michael S. Tsirkin wrote: On Fri, Jun 10, 2011 at 02:55:35PM +0200, Hannes Reinecke wrote: Device operation: request queues The driver queues requests to an arbitrary request queue, and they are used by the device on that same queue. What about request ordering? If requests are placed on arbitrary queues you'll inevitably run on locking issues to ensure strict request ordering. I would add here: If a device uses more than one queue it is the responsibility of the device to ensure strict request ordering. Maybe I misunderstand - how can this be the responsibility of the device if the device does not get the information about the original ordering of the requests? For example, if the driver is crazy enough to put all write requests on one queue and all barriers on another one, how is the device supposed to ensure ordering? Which is exactly the problem I was referring to. When using more than one channel the request ordering _as seen by the initiator_ has to be preserved. This is quite hard to do from a device's perspective; it might be able to process the requests _in the order_ they've arrived, but it won't be able to figure out the latency of each request, ie how it'll take the request to be delivered to the initiator. What we need to do here is to ensure that virtio will deliver the requests in-order across all virtqueues. Not sure whether it does this already. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: virtio scsi host draft specification, v3
On 06/29/2011 12:07 PM, Christoph Hellwig wrote: On Wed, Jun 29, 2011 at 10:39:42AM +0100, Stefan Hajnoczi wrote: I think we're missing a level of addressing. We need the ability to talk to multiple target ports in order for list target ports to make sense. Right now there is one implicit target that handles all commands. That means there is one fixed I_T Nexus. If we introduce list target ports we also need a way to say This CDB is destined for target port #0. Then it is possible to enumerate target ports and address targets independently of the LUN field in the CDB. I'm pretty sure this is also how SAS and other transports work. In their framing they include the target port. Yes, exactly. Hierachial LUNs are a nasty fringe feature that we should avoid as much as possible, that is for everything but IBM vSCSI which is braindead enough to force them. Yep. The question is whether we really need to support multiple targets on a virtio-scsi adapter or not. If you are selectively mapping LUNs that the guest may access, then multiple targets are not necessary. If we want to do pass-through of the entire SCSI bus then we need multiple targets but I'm not sure if there are other challenges like dependencies on the transport (Fibre Channel, SAS, etc) which make it impossible to pass through bus-level access? I don't think bus-level pass through is either easily possible nor desirable. What multiple targets are useful for is allowing more virtual disks than we have virtual PCI slots. We could do this by supporting multiple LUNs, but given that many SCSI ressources are target-based doing multiple targets most likely is the more scabale and more logical variant. E.g. we could much more easily have one virtqueue per target than per LUN. The general idea here is that we can support NPIV. With NPIV we'll have several scsi_hosts, each of which is assigned a different set of LUNs by the array. With virtio we need to able to react on LUN remapping on the array side, ie we need to be able to issue a 'REPORT LUNS' command and add/remove LUNs on the fly. This means we have to expose the scsi_host in some way via virtio. This is impossible with a one-to-one mapping between targets and LUNs. The actual bus-level pass-through will be just on the SCSI layer, ie 'REPORT LUNS' should be possible. If and how we do a LUN remapping internally on the host is a totally different matter. Same goes for the transport details; I doubt we will expose all the dingy details of the various transports, but rather restrict ourselves to an abstract transport. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: binding/unbinding devices to vfio-pci
On 07/02/2013 05:13 PM, Yoder Stuart-B08248 wrote: -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Tuesday, July 02, 2013 9:46 AM To: Yoder Stuart-B08248 Cc: kvm@vger.kernel.org list; Alexander Graf; Bhushan Bharat-R65777; a.mota...@virtualopensystems.com; virtualizat...@lists.linux-foundation.org Subject: Re: binding/unbinding devices to vfio-pci On Tue, 2013-07-02 at 14:15 +, Yoder Stuart-B08248 wrote: Alex, I'm trying to think through how binding/unbinding of devices will work with VFIO for platform devices and have a couple of questions about how vfio-pci works. When you bind a device to vfio-pci, e.g.: # echo 1102 0002 /sys/bus/pci/drivers/vfio-pci/new_id ...I understand that the echo into 'new_id' tells the vfio pci driver that it now handles the specified PCI ID. But now there are 2 drivers that handle that PCI ID, the original host driver and vfio-pci. Say that you hotplug a PCI device that matches that ID. Which of the 2 drivers are going to get bound to the device? Also, if you unbind a device from vfio-pci and want to bind it again to the normal host driver you would just echo the full device info into the 'bind' sysfs file for the host driver, right? echo :06:0d.0 /sys/bus/pci/drivers/... Hi Stuart, The driver binding interface is far from perfect. In your scenario where you've added the ID for one device, then hotplug another device with the same ID, the results are indeterminate. Both vfio-pci and the host driver, assuming it's still loaded, can claim the device, it's just a matter of which gets probed first. Generally that window should be very short though. To bind a device, the user should do: 1) echo :bb:dd.f /sys/bus/pci/devices/:bb:dd.f/driver/unbind 2) echo /sys/bus/pci/drivers/vfio-pci/new_id 3) echo :bb:dd.f /sys/bus/pci/drivers/vfio-pci/bind 4) echo /sys/bus/pci/drivers/vfio-pci/remove_id There are actually a number of ways you can do this and the default autoprobe behavior really makes step 3) unnecessary as the driver core will probe any unbound devices as soon as a new_id is added to vfio-pci. That can be changed by: # echo 0 /sys/bus/pci/drivers_autoprobe But then we have to worry about races from any devices that might have been hotplugged in the interim. But, even apart from hot-plugged devices, what about the device we just unbound? There are now 2 host drivers that can handle the device when the autoprobe happens. Is it just luck that vfio-pci is the one that gets the device? Probably the best way of handling this would be to disallow autobinding for vfio in general. Then the 'normal' driver would be bound to the device, and vfio must be enabled via manual binding. Much like it is today. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: updated: kvm PCI todo wiki
On 08/21/2013 12:48 PM, Michael S. Tsirkin wrote: Hey guys, I've put up a wiki page with a kvm PCI todo list, mainly to avoid effort duplication, but also in the hope to draw attention to what I think we should try addressing in KVM: http://www.linux-kvm.org/page/PCITodo This page could cover all PCI related activity in KVM, it is very incomplete. We should probably add e.g. IOMMU related stuff. Note: if there's no developer listed for an item, this just means I don't know of anyone actively working on an issue at the moment, not that no one intends to. I would appreciate it if others working on one of the items on this list would add their names so we can communicate better. If others like this wiki page, please go ahead and add stuff you are working on if any. It would be especially nice to add testing projects. Also, feel free to add links to bugzillas items. On a related note, did anyone ever tried to test MSI / MSI-X with a windows guest? I've tried to enable it for virtio but for some reason Windows didn't wanted to enable it. AHCI was even worse; the stock Windows version doesn't support MSI and the Intel one doesn't like our implementation :-(. Anyone ever managed to get this to work? If not it'd be a good topic for the wiki ... Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] virtio-scsi: Change sense buffer size to 252
On 03/06/2014 11:09 AM, Paolo Bonzini wrote:
Il 06/03/2014 09:47, Fam Zheng ha scritto:
According to SPC-4, section 4.5.2.1, 252 is the limit of sense
data. So
increase the value.
Signed-off-by: Fam Zheng f...@redhat.com
---
include/linux/virtio_scsi.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/virtio_scsi.h
b/include/linux/virtio_scsi.h
index 4195b97..a437f7f 100644
--- a/include/linux/virtio_scsi.h
+++ b/include/linux/virtio_scsi.h
@@ -28,7 +28,7 @@
#define _LINUX_VIRTIO_SCSI_H
#define VIRTIO_SCSI_CDB_SIZE 32
-#define VIRTIO_SCSI_SENSE_SIZE 96
+#define VIRTIO_SCSI_SENSE_SIZE 252
/* SCSI command request, followed by data-out */
struct virtio_scsi_cmd_req {
Hi Fam, how did you test this?
Is there a specific reason _not_ to use the linux default?
The SCSI stack typically limits the sense code to
SCSI_SENSE_BUFFERSIZE, so using other values have a
limited sense.
Literally :-)
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] scsi: Change sense buffer size to 252
On 03/14/2014 07:00 AM, Fam Zheng wrote:
According to SPC-4, section 4.5.2.1, 252 is the limit of sense data. So
increase the values.
Tested by hacking QEMU to fake virtio-scsi request sense len to 252.
Without this patch the driver stops working immediately when it gets the
request.
Signed-off-by: Fam Zheng f...@redhat.com
---
include/linux/virtio_scsi.h | 2 +-
include/scsi/scsi_cmnd.h| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_scsi.h b/include/linux/virtio_scsi.h
index 4195b97..a437f7f 100644
--- a/include/linux/virtio_scsi.h
+++ b/include/linux/virtio_scsi.h
@@ -28,7 +28,7 @@
#define _LINUX_VIRTIO_SCSI_H
#define VIRTIO_SCSI_CDB_SIZE 32
-#define VIRTIO_SCSI_SENSE_SIZE 96
+#define VIRTIO_SCSI_SENSE_SIZE 252
/* SCSI command request, followed by data-out */
struct virtio_scsi_cmd_req {
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 91558a1..a64dac03 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -104,7 +104,7 @@ struct scsi_cmnd {
struct request *request;/* The command we are
working on */
-#define SCSI_SENSE_BUFFERSIZE96
+#define SCSI_SENSE_BUFFERSIZE252
unsigned char *sense_buffer;
/* obtained by REQUEST SENSE when
* CHECK CONDITION is received on original
Not without careful review.
Blindly increasing the buffersize is not a good idea; this define is
used at several locations and even within the drivers themselves.
So we cannot just increase the define for the SCSI stack.
And, btw, so far I haven't come across any issue where a sense
buffer overflow occurred. We first need to implement a proper sense
code handling (descriptor sense parsing etc) before we need to worry
about this.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/4] scsi: remove old-style type names from sg.h
On 09/25/2015 11:27 AM, Paolo Bonzini wrote:
> These will not be exported by the new linux/sg.h header, and scsi/sg.h will
> not have any user API after linux/sg.h is created. Since they have no
> user in the kernel, they can be zapped.
>
> Cc: James Bottomley <jbottom...@parallels.com>
> Cc: Christoph Hellwig <h...@lst.de>
> Cc: linux-s...@vger.kernel.org
> Reviewed-by: Bart Van Assche <bart.vanass...@sandisk.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> include/scsi/sg.h | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/include/scsi/sg.h b/include/scsi/sg.h
> index 3afec7032448..370c78c37926 100644
> --- a/include/scsi/sg.h
> +++ b/include/scsi/sg.h
> @@ -207,12 +207,6 @@ typedef struct sg_req_info { /* used by
> SG_GET_REQUEST_TABLE ioctl() */
>
> #define SG_BIG_BUFF SG_DEF_RESERVED_SIZE/* for backward compatibility */
>
> -/* Alternate style type names, "..._t" variants preferred */
> -typedef struct sg_io_hdr Sg_io_hdr;
> -typedef struct sg_io_vec Sg_io_vec;
> -typedef struct sg_scsi_id Sg_scsi_id;
> -typedef struct sg_req_info Sg_req_info;
> -
>
> /* vvvvvv */
> /* The older SG interface based on the 'sg_header' structure follows. */
>
Reviewed-by: Hannes Reinecke <h...@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] scsi: move all obsolete ioctls to scsi_ioctl.h
On 09/25/2015 11:27 AM, Paolo Bonzini wrote: > Some are in scsi.h. Keep them together in preparation for exposing them > in UAPI headers. > > Cc: James Bottomley <jbottom...@parallels.com> > Cc: Christoph Hellwig <h...@lst.de> > Cc: linux-s...@vger.kernel.org > Reviewed-by: Bart Van Assche <bart.vanass...@sandisk.com> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- > include/scsi/scsi.h | 20 > include/scsi/scsi_ioctl.h | 20 > 2 files changed, 20 insertions(+), 20 deletions(-) > [ .. ] Reviewed-by: Hannes Reinecke <h...@suse.com> Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/4] scsi: cleanup scsi/scsi_ioctl.h
On 09/25/2015 11:27 AM, Paolo Bonzini wrote:
> SCSI_REMOVAL_* goes together with other SCSI command constants in
> include/scsi/scsi.h. It is also used outside the implementation
> of the ioctls (and it is not part of the user API).
>
> scsi_fctargaddress/Scsi_FCTargAddress has had no in-tree use since
> commit ca61f10ab2b8 ("[SCSI] remove broken driver cpqfc", 2005-10-29).
> Remove it, just in time for the the tenth anniversary of its demise.
>
> Cc: James Bottomley <jbottom...@parallels.com>
> Cc: Christoph Hellwig <h...@lst.de>
> Cc: linux-s...@vger.kernel.org
> Reviewed-by: Bart Van Assche <bart.vanass...@sandisk.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> include/scsi/scsi.h | 6 ++
> include/scsi/scsi_ioctl.h | 8 ----
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
[ .. ]
Reviewed-by: Hannes Reinecke <h...@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] scsi: provide UAPI version of scsi/sg.h and scsi/scsi_ioctl.h
On 09/25/2015 11:27 AM, Paolo Bonzini wrote: > Provide a UAPI version of the header in the kernel, making it easier > for interested projects to use an up-to-date version of the header. > > The new headers are placed under uapi/linux/ so as not to conflict > with the glibc-provided headers in /usr/include/scsi. > > /dev/sgN default values are implementation aspects, and are moved to > drivers/scsi/sg.c instead (together with e.g. SG_ALLOW_DIO_DEF). > However, SG_SCATTER_SZ is used by Wine so it is kept in linux/sg.h > SG_MAX_QUEUE could also be useful. > > struct scsi_ioctl_command and struct scsi_idlun used to be under > "#ifdef __KERNEL__", but they are actually useful for userspace as > well. Add them to the new header. > > Cc: James Bottomley <jbottom...@parallels.com> > Cc: Christoph Hellwig <h...@lst.de> > Cc: linux-s...@vger.kernel.org > Cc: Bart Van Assche <bart.vanass...@sandisk.com> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- [ .. ] Reviewed-by: Hannes Reinecke <h...@suse.com> Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
