From: Boqun Feng <boqun.f...@gmail.com> Sent: Thursday, September 10, 2020 7:35
AM
>
> Hyper-V always use 4k page size (HV_HYP_PAGE_SIZE), so when
> communicating with Hyper-V, a guest should always use HV_HYP_PAGE_SIZE
> as the unit for page related data. For storvsc, the data is
> vmbus_packet_mpb_array. And since in scsi_cmnd, sglist of pages (in unit
> of PAGE_SIZE) is used, we need convert pages in the sglist of scsi_cmnd
> into Hyper-V pages in vmbus_packet_mpb_array.
>
> This patch does the conversion by dividing pages in sglist into Hyper-V
> pages, offset and indexes in vmbus_packet_mpb_array are recalculated
> accordingly.
>
> Signed-off-by: Boqun Feng <boqun.f...@gmail.com>
> ---
> drivers/scsi/storvsc_drv.c | 54 +++++++++++++++++++++++++++++++++-----
> 1 file changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 8f5f5dc863a4..119b76ca24a1 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1739,23 +1739,63 @@ static int storvsc_queuecommand(struct Scsi_Host
> *host, struct
> scsi_cmnd *scmnd)
> payload_sz = sizeof(cmd_request->mpb);
>
> if (sg_count) {
> - if (sg_count > MAX_PAGE_BUFFER_COUNT) {
> + unsigned int hvpgoff = 0;
> + unsigned long hvpg_offset = sgl->offset & ~HV_HYP_PAGE_MASK;
This is a minor nit. The above expression uses sgl->offset. Code below uses
sgl[0].offset. They're the same but the inconsistency sticks out a bit.
> + unsigned int hvpg_count = HVPFN_UP(hvpg_offset + length);
> + u64 hvpfn;
>
> - payload_sz = (sg_count * sizeof(u64) +
> + if (hvpg_count > MAX_PAGE_BUFFER_COUNT) {
> +
> + payload_sz = (hvpg_count * sizeof(u64) +
> sizeof(struct vmbus_packet_mpb_array));
> payload = kzalloc(payload_sz, GFP_ATOMIC);
> if (!payload)
> return SCSI_MLQUEUE_DEVICE_BUSY;
> }
>
> + /*
> + * sgl is a list of PAGEs, and payload->range.pfn_array
> + * expects the page number in the unit of HV_HYP_PAGE_SIZE (the
> + * page size that Hyper-V uses, so here we need to divide PAGEs
> + * into HV_HYP_PAGE in case that PAGE_SIZE > HV_HYP_PAGE_SIZE.
> + */
> payload->range.len = length;
> - payload->range.offset = sgl[0].offset;
> + payload->range.offset = sgl[0].offset & ~HV_HYP_PAGE_MASK;
Another nit. The right hand side of the above assignment is already calculated
as
hvpg_offset.
Nits aside,
Reviewed-by: Michael Kelley <mikel...@microsoft.com>
> + hvpgoff = sgl[0].offset >> HV_HYP_PAGE_SHIFT;
>
> cur_sgl = sgl;
> - for (i = 0; i < sg_count; i++) {
> - payload->range.pfn_array[i] =
> - page_to_pfn(sg_page((cur_sgl)));
> - cur_sgl = sg_next(cur_sgl);
> + for (i = 0; i < hvpg_count; i++) {
> + /*
> + * 'i' is the index of hv pages in the payload and
> + * 'hvpgoff' is the offset (in hv pages) of the first
> + * hv page in the the first page. The relationship
> + * between the sum of 'i' and 'hvpgoff' and the offset
> + * (in hv pages) in a payload page ('hvpgoff_in_page')
> + * is as follow:
> + *
> + * |------------------ PAGE -------------------|
> + * | NR_HV_HYP_PAGES_IN_PAGE hvpgs in total |
> + * |hvpg|hvpg| ... |hvpg|... |hvpg|
> + * ^ ^ ^
> ^
> + * +-hvpgoff-+
> +-hvpgoff_in_page-+
> + * ^
> |
> + * +--------------------- i
> ---------------------------+
> + */
> + unsigned int hvpgoff_in_page =
> + (i + hvpgoff) % NR_HV_HYP_PAGES_IN_PAGE;
> +
> + /*
> + * Two cases that we need to fetch a page:
> + * 1) i == 0, the first step or
> + * 2) hvpgoff_in_page == 0, when we reach the boundary
> + * of a page.
> + */
> + if (hvpgoff_in_page == 0 || i == 0) {
> + hvpfn = page_to_hvpfn(sg_page(cur_sgl));
> + cur_sgl = sg_next(cur_sgl);
> + }
> +
> + payload->range.pfn_array[i] = hvpfn + hvpgoff_in_page;
> }
> }
>
> --
> 2.28.0
