RE: [PATCH V5 11/12] scsi: storvsc: Add Isolation VM support for storvsc driver

[Michael Kelley <mikelley@xxxxxxxxxxxxx>]

From: Tianyu Lan <ltykernel@xxxxxxxxx> Sent: Tuesday, September 14, 2021 6:39 AM
> 
> In Isolation VM, all shared memory with host needs to mark visible
> to host via hvcall. vmbus_establish_gpadl() has already done it for
> storvsc rx/tx ring buffer. The page buffer used by vmbus_sendpacket_
> mpb_desc() still needs to be handled. Use DMA API(scsi_dma_map/unmap)
> to map these memory during sending/receiving packet and return swiotlb
> bounce buffer dma address. In Isolation VM, swiotlb  bounce buffer is
> marked to be visible to host and the swiotlb force mode is enabled.
> 
> Set device's dma min align mask to HV_HYP_PAGE_SIZE - 1 in order to
> keep the original data offset in the bounce buffer.
> 
> Signed-off-by: Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx>
> ---
> Change since v4:
>       * use scsi_dma_map/unmap() instead of dma_map/unmap_sg()
>       * Add deleted comments back.
>       * Fix error calculation of  hvpnfs_to_add
> 
> Change since v3:
>       * Rplace dma_map_page with dma_map_sg()
>       * Use for_each_sg() to populate payload->range.pfn_array.
>       * Remove storvsc_dma_map macro
> ---
>  drivers/hv/vmbus_drv.c     |  1 +
>  drivers/scsi/storvsc_drv.c | 24 +++++++++++++++---------
>  include/linux/hyperv.h     |  1 +
>  3 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index b0be287e9a32..9c53f823cde1 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -2121,6 +2121,7 @@ int vmbus_device_register(struct hv_device 
> *child_device_obj)
>       hv_debug_add_dev_dir(child_device_obj);
> 
>       child_device_obj->device.dma_mask = &vmbus_dma_mask;
> +     child_device_obj->device.dma_parms = &child_device_obj->dma_parms;
>       return 0;
> 
>  err_kset_unregister:
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index ebbbc1299c62..d10b450bcf0c 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -21,6 +21,8 @@
>  #include <linux/device.h>
>  #include <linux/hyperv.h>
>  #include <linux/blkdev.h>
> +#include <linux/dma-mapping.h>
> +
>  #include <scsi/scsi.h>
>  #include <scsi/scsi_cmnd.h>
>  #include <scsi/scsi_host.h>
> @@ -1322,6 +1324,7 @@ static void storvsc_on_channel_callback(void *context)
>                                       continue;
>                               }
>                               request = (struct storvsc_cmd_request 
> *)scsi_cmd_priv(scmnd);
> +                             scsi_dma_unmap(scmnd);
>                       }
> 
>                       storvsc_on_receive(stor_device, packet, request);
> @@ -1735,7 +1738,6 @@ static int storvsc_queuecommand(struct Scsi_Host *host, 
> struct scsi_cmnd *scmnd)
>       struct hv_host_device *host_dev = shost_priv(host);
>       struct hv_device *dev = host_dev->dev;
>       struct storvsc_cmd_request *cmd_request = scsi_cmd_priv(scmnd);
> -     int i;
>       struct scatterlist *sgl;
>       unsigned int sg_count;
>       struct vmscsi_request *vm_srb;
> @@ -1817,10 +1819,11 @@ static int storvsc_queuecommand(struct Scsi_Host 
> *host, struct scsi_cmnd *scmnd)
>       payload_sz = sizeof(cmd_request->mpb);
> 
>       if (sg_count) {
> -             unsigned int hvpgoff, hvpfns_to_add;
>               unsigned long offset_in_hvpg = offset_in_hvpage(sgl->offset);
>               unsigned int hvpg_count = HVPFN_UP(offset_in_hvpg + length);
> -             u64 hvpfn;
> +             struct scatterlist *sg;
> +             unsigned long hvpfn, hvpfns_to_add;
> +             int j, i = 0;
> 
>               if (hvpg_count > MAX_PAGE_BUFFER_COUNT) {
> 
> @@ -1834,8 +1837,11 @@ static int storvsc_queuecommand(struct Scsi_Host 
> *host, struct scsi_cmnd *scmnd)
>               payload->range.len = length;
>               payload->range.offset = offset_in_hvpg;
> 
> +             sg_count = scsi_dma_map(scmnd);
> +             if (sg_count < 0)
> +                     return SCSI_MLQUEUE_DEVICE_BUSY;
> 
> -             for (i = 0; sgl != NULL; sgl = sg_next(sgl)) {
> +             for_each_sg(sgl, sg, sg_count, j) {
>                       /*
>                        * Init values for the current sgl entry. hvpgoff
>                        * and hvpfns_to_add are in units of Hyper-V size

Nit:  The above comment is now out-of-date because hvpgoff has
been removed.

> @@ -1845,10 +1851,9 @@ static int storvsc_queuecommand(struct Scsi_Host 
> *host, struct scsi_cmnd *scmnd)
>                        * even on other than the first sgl entry, provided
>                        * they are a multiple of PAGE_SIZE.
>                        */
> -                     hvpgoff = HVPFN_DOWN(sgl->offset);
> -                     hvpfn = page_to_hvpfn(sg_page(sgl)) + hvpgoff;
> -                     hvpfns_to_add = HVPFN_UP(sgl->offset + sgl->length) -
> -                                             hvpgoff;
> +                     hvpfn = HVPFN_DOWN(sg_dma_address(sg));
> +                     hvpfns_to_add = HVPFN_UP(sg_dma_address(sg) +
> +                                              sg_dma_len(sg)) - hvpfn;

Good.  This looks correct now.

> 
>                       /*
>                        * Fill the next portion of the PFN array with
> @@ -1858,7 +1863,7 @@ static int storvsc_queuecommand(struct Scsi_Host *host, 
> struct scsi_cmnd *scmnd)
>                        * the PFN array is filled.
>                        */
>                       while (hvpfns_to_add--)
> -                             payload->range.pfn_array[i++] = hvpfn++;
> +                             payload->range.pfn_array[i++] = hvpfn++;
>               }
>       }
> 
> @@ -2002,6 +2007,7 @@ static int storvsc_probe(struct hv_device *device,
>       stor_device->vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
>       spin_lock_init(&stor_device->lock);
>       hv_set_drvdata(device, stor_device);
> +     dma_set_min_align_mask(&device->device, HV_HYP_PAGE_SIZE - 1);
> 
>       stor_device->port_number = host->host_no;
>       ret = storvsc_connect_to_vsp(device, storvsc_ringbuffer_size, is_fc);
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index bb1a1519b93a..c94c534a944e 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1274,6 +1274,7 @@ struct hv_device {
> 
>       struct vmbus_channel *channel;
>       struct kset          *channels_kset;
> +     struct device_dma_parameters dma_parms;
> 
>       /* place holder to keep track of the dir for hv device in debugfs */
>       struct dentry *debug_dir;
> --
> 2.25.1