mpt2sas: T10 DIF Support - EEDP

This add support for type 1 and 3 DIF support per the Oracle API.

diff -uaprN a/drivers/scsi/mpt2sas/mpt2sas_base.h b/drivers/scsi/mpt2sas/mpt2sas_base.h
@@ -61,6 +61,7 @@
 #include <scsi/scsi_tcq.h>
 #include <scsi/scsi_transport_sas.h>
 #include <scsi/scsi_dbg.h>
+#include <scsi/scsi_eh.h>
 
 #include "mpt2sas_debug.h"
 

Eric,

+_scsih_setup_eedp(struct scsi_cmnd *scmd, Mpi2SCSIIORequest_t *mpi_request) [...]
+ /*
+ * enable ref/app/guard checking
+ * auto increment ref tag
+ */
+ mpi_request->EEDPFlags = eedp_flags |

+	    MPI2_SCSIIO_EEDPFLAGS_INC_PRI_REFTAG |
+	    MPI2_SCSIIO_EEDPFLAGS_CHECK_REFTAG |
+	    MPI2_SCSIIO_EEDPFLAGS_CHECK_APPTAG |
+	    MPI2_SCSIIO_EEDPFLAGS_CHECK_GUARD;

How can you check the app tag?

+_scsih_eedp_error_handling(struct scsi_cmnd *scmd, u16 ioc_status) [...]
+ if (scmd->sc_data_direction == DMA_TO_DEVICE) {
+ sk = ILLEGAL_REQUEST;
+ host_byte = DID_ABORT;
+ } else {
+ sk = ABORTED_COMMAND;
+ host_byte = DID_OK;
+ }

ILLEGAL REQUEST is used when the HBA detects a mismatch between data and protection information. If you are not receiving PI from the OS (WRITE INSERT) then the HBA can't detect an integrity error. So in the DMA_TO_DEVICE case only the target can catch the error (and it will return ABORTED_COMMAND).

For READ, both the drive firmware and the HBA can ostensibly check that the data buffers and PI match.

Are your EEDP error flags only set when the HBA firmware detects a problem or also when the drive returns ABORTED COMMAND/0x10?

I.e. do we get double failure scenarios (both drive and HBA detect the mismatch)?

+ scsi_host_set_prot(shost, SHOST_DIF_TYPE1_PROTECTION
+ | SHOST_DIF_TYPE3_PROTECTION);

How do you handle Type 3? The application tag and reference tags are both defined as being opaque storage for Type 3.

If I understand correctly, your firmware only deals with the Type 2 usage model of the application tag. And Type 1 + 2 usage of the reference tag.

+ scsi_host_set_guard(shost, SHOST_DIX_GUARD_CRC);

Technically speaking that is only required for DIX exchange. But setting it doesn't hurt.

Martin K. Petersen	Oracle Linux Engineering

The SAS controller firmware checks the tags, not device driver. What this code is doing is setting flags in the SCSI_IO Request message that is being sent to controller firmware.

Your right.

I wrote is a generic function based on chapter 3.3 in your linux-hba.pdf doc found at http://oss.oracle.com/projects/data-integrity/dist/documentation/linux-hba.pdf.

Obviously the only errors the current implementation will only be flaged on READs.

My line of thinking was I write the code so it won't need to be rewrote later when DIX support is added for SAS3.0.

Yes, I was told by the LSI software test lab, they saw drives returning check condition with these ref tag errors. I wasn't able to repro that. What I saw were the IOCSTATUS errors, which is what the code handles in _scsih_eedp_error_handling(). Either way, both cases are covered right?

I have a 3gb SAS drive with EEDP support, model HITACHI HUS153073VLS300. This support type 1 and 2.

I also EEDP support for target mode driver for this 6gb part, type 1, 2, and 3.

I used both to test the driver.

Yeah I know that. Since its app tag, it can be set to anything you want it to be. For our internal implementation of type 3 DIF, I'm setting the lower 32bit part of the tag to the lba. I was able to test this with a type 3 target, and it work fine.

Our controller firmware support all three types of protection..

I wasn't sure whether it needed to be set. Our controller generates the CRC. I'm not sure whether firmware or hardware implementation.

Eric,

Eric> The SAS controller firmware checks the tags, not device driver. Eric> What this code is doing is setting flags in the SCSI_IO Request Eric> message that is being sent to controller firmware.

Yeah, I figured that much. I guess it makes sense when you treat the app tag as being constant (modulo the mask) for one request.

Eric> Obviously the only errors the current implementation will only be Eric> flaged on READs.

Eric> My line of thinking was I write the code so it won't need to be Eric> rewrote later when DIX support is added for SAS3.0.

Ok, that's fine then.

Eric> Yes, I was told by the LSI software test lab, they saw drives Eric> returning check condition with these ref tag errors. I wasn't Eric> able to repro that. What I saw were the IOCSTATUS errors, which Eric> is what the code handles in _scsih_eedp_error_handling(). Either Eric> way, both cases are covered right?

Yeah. With READ the difference is mostly academic.

Eric> Our controller firmware support all three types of protection..

So you can actually set the application tag (and reference tag) on a per-sector basis for Type 1 and 3 provided the OS sends down 520-byte sectors?

Eric> I wasn't sure whether it needed to be set. Our controller Eric> generates the CRC. I'm not sure whether firmware or hardware Eric> implementation.

Given that it sounds like your next chip rev. will use the same driver it should be ok to leave it in place.

Martin K. Petersen	Oracle Linux Engineering

I forgot to mention how they got the drives to return EEDP errors. They were using sas protocal jammers to inject errors on the bus.

We have a choice of setting the tags on a per IO request basis. The same tag can apply to all sectors, or you can set some bit telling controller firmware to increment the tag for each sector. So the bottom line, the current interface I have is not setup for setting each sector from the driver unless I send the normal data having the protection bytes already interleaved in the scatter gather list from the device driver. (its silly to do that).

So what I did is good for type1, which is set the ref tag to the lower 32bit of the lba, and the app tag to zero.

For type 3, I asked others for suggestions from a number of co-workers and the software archetic, and they said to set the tags the same way I did for type 1. Due to my limitations with controller firmware, what do you suggest for type 3?

Thats the plan I hope.

Eric,

Eric> So what I did is good for type1, which is set the ref tag to the Eric> lower 32bit of the lba, and the app tag to zero.

That's fine.

Eric> For type 3, I asked others for suggestions from a number of Eric> co-workers and the software archetic, and they said to set the Eric> tags the same way I did for type 1. Due to my limitations with Eric> controller firmware, what do you suggest for type 3?

Well, so technically you are not supposed to set the reference tag to the LBA in Type 3. The 16 bits of the app tag + the 32 bits of the ref tag are considered opaque storage.

The storage device has no way of checking neither app, nor ref with Type
3. It can only check the guard tag.

My main concern is if you always expect the ref to be lower 32 bits of LBA and have the HBA firmware verify that on READ. Because the ref tag could conceivably contain anything and the I/O might fail.

I realize this is a bit academic given that with the current setup your HBA will be doing both writing and reading. But in a multi-initiator setup or for future compatibility it might throw a wrench in the machinery to have the HBA check the ref tag on Type 3.

For Type 1 I expect you to check guard + match ref tag to CDB LBA + offset.

For Type 2 I expect you to check guard + match ref tag to E.I.LBA in 32-byte CDB + offset.

For Type 3 I expect you to check guard only.

Martin K. Petersen	Oracle Linux Engineering