On Tue, 2011-08-16 at 09:57 -0700, Roland Dreier wrote:
> From: Roland Dreier <roland@xxxxxxxxxxxxxxx>
>
> LIO core uses data_direction in the opposite way that qla2xxx needs; for
> example, to the LIO core, a READ command is getting data from the target
> so it uses DMA_FROM_DEVICE; however for the HBA to be able to DMA the
> data to respond to the READ command, the buffer should be mapped with
> DMA_TO_DEVICE (since data is flowing from memory to the HBA).
>
> This causes problems on systems with a real IOMMU; eg with VT-d
> (intel_iommu) enabled, one sees messages like
>
> DMAR:[DMA Read] Request device [06:00.0] fault addr ffe4e000
> DMAR:[fault reason 06] PTE Read access is not set
>
> and the target is not able to transfer any data.
>
> Fix this by adding a function tcm_qla2xxx_mapping_dir() that converts
> from the LIO core direction to the value that qla2xxx needs to use.
>
Ouch.. So it sounds like the dma-mapping.h macro usage in target core
is backwards for following the DMA_FROM_DEVICE -> SCSI READ and
DMA_TO_DEVICE -> SCSI WRITE as used by drivers/scsi/ for traditional
initiator mode / LLD operation.
Conceptually this makes things more confusing, but requiring each HW
target fabric driver to have to flip these around looks pretty ugly..
I'm happy to merge this as a workaround to address the DMAR exceptions
above, but need to think if it makes sense to do a wholesale conversion
of this for target core..
> Signed-off-by: Roland Dreier <roland@xxxxxxxxxxxxxxx>
> ---
> drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c | 31 ++++++++++++++++++++--
> 1 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c b/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c
> index c0c7d2e..305f554 100644
> --- a/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c
> +++ b/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c
> @@ -513,12 +513,37 @@ u32 tcm_qla2xxx_sess_get_index(struct se_session *se_sess)
> return 0;
> }
>
> +/*
> + * The LIO target core uses DMA_TO_DEVICE to mean that data is going
> + * to the target (eg handling a WRITE) and DMA_FROM_DEVICE to mean
> + * that data is coming from the target (eg handling a READ). However,
> + * this is just the opposite of what we have to tell the DMA mapping
> + * layer -- eg when handling a READ, the HBA will have to DMA the data
> + * out of memory so it can send it to the initiator, which means we
> + * need to use DMA_TO_DEVICE when we map the data.
> + */
> +static enum dma_data_direction tcm_qla2xxx_mapping_dir(struct se_cmd *se_cmd)
> +{
> + if (se_cmd->t_tasks_bidi)
> + return DMA_BIDIRECTIONAL;
> +
Mmmm, I don't think this is correct. It's my understanding that
DMA_BIDIRECTIONAL is different from SCSI bidi operation, and that the
individual bidi payloads should be retaining their individual
DMA_FROM_DEVICE and DMA_TO_DEVICE usage.. (Boaz CC'ed)
> + switch (se_cmd->data_direction) {
> + case DMA_TO_DEVICE:
> + return DMA_FROM_DEVICE;
> + case DMA_FROM_DEVICE:
> + return DMA_TO_DEVICE;
> + case DMA_NONE:
> + default:
> + return DMA_NONE;
> + }
> +}
> +
> int tcm_qla2xxx_write_pending(struct se_cmd *se_cmd)
> {
> struct qla_tgt_cmd *cmd = container_of(se_cmd, struct qla_tgt_cmd, se_cmd);
>
> cmd->bufflen = se_cmd->data_length;
> - cmd->dma_data_direction = se_cmd->data_direction;
> + cmd->dma_data_direction = tcm_qla2xxx_mapping_dir(se_cmd);
> /*
> * Setup the struct se_task->task_sg[] chained SG list
> */
> @@ -720,7 +745,7 @@ int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd)
> struct qla_tgt_cmd *cmd = container_of(se_cmd, struct qla_tgt_cmd, se_cmd);
>
> cmd->bufflen = se_cmd->data_length;
> - cmd->dma_data_direction = se_cmd->data_direction;
> + cmd->dma_data_direction = tcm_qla2xxx_mapping_dir(se_cmd);
> cmd->aborted = atomic_read(&se_cmd->t_transport_aborted);
> /*
> * Setup the struct se_task->task_sg[] chained SG list
> @@ -754,7 +779,7 @@ int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd)
> cmd->sg = NULL;
> cmd->sg_cnt = 0;
> cmd->offset = 0;
> - cmd->dma_data_direction = se_cmd->data_direction;
> + cmd->dma_data_direction = tcm_qla2xxx_mapping_dir(se_cmd);
> cmd->aborted = atomic_read(&se_cmd->t_transport_aborted);
>
> if (se_cmd->data_direction == DMA_FROM_DEVICE) {
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
