Re: [win-pv-devel] [PATCH 05/10] Move scatter gather list parsing to adapter.c

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel [mailto:win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On
> Behalf Of owen.smith@xxxxxxxxxx
> Sent: 23 June 2017 13:49
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Owen Smith <owen.smith@xxxxxxxxxx>
> Subject: [win-pv-devel] [PATCH 05/10] Move scatter gather list parsing to
> adapter.c
> 
> From: Owen Smith <owen.smith@xxxxxxxxxx>
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>

Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

> ---
>  src/xenvbd/adapter.c |  62 +++++++++++++++++++++++++++
>  src/xenvbd/adapter.h |   9 ++++
>  src/xenvbd/srbext.h  |   4 ++
>  src/xenvbd/target.c  | 116 
> +++++++++++++--------------------------------------
>  4 files changed, 104 insertions(+), 87 deletions(-)
> 
> diff --git a/src/xenvbd/adapter.c b/src/xenvbd/adapter.c
> index bc93037..14e9ae6 100644
> --- a/src/xenvbd/adapter.c
> +++ b/src/xenvbd/adapter.c
> @@ -1434,6 +1434,66 @@ AdapterSetDeviceQueueDepth(
>                  TargetId);
>  }
> 
> +PFN_NUMBER
> +AdapterGetNextSGEntry(
> +    IN  PXENVBD_ADAPTER Adapter,
> +    IN  PXENVBD_SRBEXT  SrbExt,
> +    IN  ULONG           Existing,
> +    OUT PULONG          Offset,
> +    OUT PULONG          Length
> +    )
> +{
> +    PSTOR_SCATTER_GATHER_LIST       SGList = SrbExt->SGList;
> +    PSTOR_SCATTER_GATHER_ELEMENT    SGElement;
> +    STOR_PHYSICAL_ADDRESS           PhysAddr;
> +
> +    UNREFERENCED_PARAMETER(Adapter);
> +
> +    ASSERT3P(SGList, !=, NULL);
> +    ASSERT3U(SrbExt->SGIndex, <, SGList->NumberOfElements);
> +
> +    SGElement = &SGList->List[SrbExt->SGIndex];
> +
> +    PhysAddr.QuadPart = SGElement->PhysicalAddress.QuadPart + SrbExt-
> >SGOffset;
> +    *Offset           = (ULONG)(PhysAddr.QuadPart & (PAGE_SIZE - 1));
> +    *Length           = __min(PAGE_SIZE - *Offset - Existing, 
> SGElement->Length
> - SrbExt->SGOffset);
> +
> +    ASSERT3U(*Length, <=, PAGE_SIZE);
> +    ASSERT3U(SrbExt->SGOffset, <, SGElement->Length);
> +
> +    SrbExt->SGOffset += *Length;
> +    if (SrbExt->SGOffset >= SGElement->Length) {
> +        SrbExt->SGIndex++;
> +        SrbExt->SGOffset = 0;
> +    }
> +
> +    return (PFN_NUMBER)(PhysAddr.QuadPart >> PAGE_SHIFT);
> +}
> +
> +static FORCEINLINE VOID
> +AdapterPullupSrb(
> +    IN  PXENVBD_ADAPTER     Adapter,
> +    IN  PSCSI_REQUEST_BLOCK Srb
> +    )
> +{
> +    PXENVBD_SRBEXT          SrbExt = Srb->SrbExtension;
> +    UCHAR                   Operation;
> +
> +    Operation = Cdb_OperationEx(Srb);
> +    switch (Operation) {
> +    case SCSIOP_READ:
> +    case SCSIOP_WRITE:
> +        SrbExt->SGList = StorPortGetScatterGatherList(Adapter, Srb);
> +        ASSERT3P(SrbExt->SGList, !=, NULL);
> +        ASSERT3U(((PSTOR_SCATTER_GATHER_LIST)SrbExt->SGList)-
> >NumberOfElements, !=, 0);
> +        ASSERT3U(((PSTOR_SCATTER_GATHER_LIST)SrbExt->SGList)-
> >NumberOfElements, <=, XENVBD_MAX_PAGES_PER_SRB);
> +        break;
> +
> +    default:
> +        break;
> +    }
> +}
> +
>  static VOID
>  AdapterUnplugRequest(
>      IN  PXENVBD_ADAPTER Adapter,
> @@ -1725,6 +1785,8 @@ AdapterHwBuildIo(
> 
>      switch (Srb->Function) {
>      case SRB_FUNCTION_EXECUTE_SCSI:
> +        AdapterPullupSrb(Adapter, Srb);
> +        // Intentional fall through
>      case SRB_FUNCTION_RESET_DEVICE:
>      case SRB_FUNCTION_FLUSH:
>      case SRB_FUNCTION_SHUTDOWN:
> diff --git a/src/xenvbd/adapter.h b/src/xenvbd/adapter.h
> index a398b75..ce0c794 100644
> --- a/src/xenvbd/adapter.h
> +++ b/src/xenvbd/adapter.h
> @@ -82,6 +82,15 @@ AdapterSetDeviceQueueDepth(
>      IN  ULONG           TargetId
>      );
> 
> +extern PFN_NUMBER
> +AdapterGetNextSGEntry(
> +    IN  PXENVBD_ADAPTER Adapter,
> +    IN  PXENVBD_SRBEXT  SrbExt,
> +    IN  ULONG           Existing,
> +    OUT PULONG          Offset,
> +    OUT PULONG          Length
> +    );
> +
>  extern NTSTATUS
>  AdapterDispatchPnp(
>      IN  PXENVBD_ADAPTER Adapter,
> diff --git a/src/xenvbd/srbext.h b/src/xenvbd/srbext.h
> index 6b41475..c5ecdb6 100644
> --- a/src/xenvbd/srbext.h
> +++ b/src/xenvbd/srbext.h
> @@ -90,6 +90,10 @@ typedef struct _XENVBD_SRBEXT {
>      PSCSI_REQUEST_BLOCK     Srb;
>      LIST_ENTRY              Entry;
>      LONG                    Count;
> +
> +    PVOID                   SGList;
> +    ULONG                   SGIndex;
> +    ULONG                   SGOffset;
>  } XENVBD_SRBEXT, *PXENVBD_SRBEXT;
> 
>  FORCEINLINE PXENVBD_SRBEXT
> diff --git a/src/xenvbd/target.c b/src/xenvbd/target.c
> index 0fe8889..c9cb803 100644
> --- a/src/xenvbd/target.c
> +++ b/src/xenvbd/target.c
> @@ -49,18 +49,6 @@
>  #include <suspend_interface.h>
>  #include <stdlib.h>
> 
> -typedef struct _XENVBD_SG_LIST {
> -    // SGList from SRB
> -    PSTOR_SCATTER_GATHER_LIST   SGList;
> -    // "current" values
> -    STOR_PHYSICAL_ADDRESS       PhysAddr;
> -    ULONG                       PhysLen;
> -    // iteration
> -    ULONG                       Index;
> -    ULONG                       Offset;
> -    ULONG                       Length;
> -} XENVBD_SG_LIST, *PXENVBD_SG_LIST;
> -
>  #define TARGET_SIGNATURE           'odpX'
> 
>  typedef struct _XENVBD_LOOKASIDE {
> @@ -732,22 +720,6 @@ __Operation(
>      }
>  }
> 
> -static FORCEINLINE ULONG
> -__Offset(
> -    __in STOR_PHYSICAL_ADDRESS   PhysAddr
> -    )
> -{
> -    return (ULONG)(PhysAddr.QuadPart & (PAGE_SIZE - 1));
> -}
> -
> -static FORCEINLINE PFN_NUMBER
> -__Phys2Pfn(
> -    __in STOR_PHYSICAL_ADDRESS   PhysAddr
> -    )
> -{
> -    return (PFN_NUMBER)(PhysAddr.QuadPart >> PAGE_SHIFT);
> -}
> -
>  static FORCEINLINE MM_PAGE_PRIORITY
>  __TargetPriority(
>      __in PXENVBD_TARGET             Target
> @@ -763,42 +735,6 @@ __TargetPriority(
>  }
> 
>  static FORCEINLINE VOID
> -SGListGet(
> -    IN OUT  PXENVBD_SG_LIST         SGList
> -    )
> -{
> -    PSTOR_SCATTER_GATHER_ELEMENT    SGElement;
> -
> -    ASSERT3U(SGList->Index, <, SGList->SGList->NumberOfElements);
> -
> -    SGElement = &SGList->SGList->List[SGList->Index];
> -
> -    SGList->PhysAddr.QuadPart = SGElement->PhysicalAddress.QuadPart +
> SGList->Offset;
> -    SGList->PhysLen           = __min(PAGE_SIZE - __Offset(SGList->PhysAddr) 
> -
> SGList->Length, SGElement->Length - SGList->Offset);
> -
> -    ASSERT3U(SGList->PhysLen, <=, PAGE_SIZE);
> -    ASSERT3U(SGList->Offset, <, SGElement->Length);
> -
> -    SGList->Length = SGList->PhysLen; // gets reset every time for Granted,
> every 1or2 times for Bounced
> -    SGList->Offset = SGList->Offset + SGList->PhysLen;
> -    if (SGList->Offset >= SGElement->Length) {
> -        SGList->Index  = SGList->Index + 1;
> -        SGList->Offset = 0;
> -    }
> -}
> -
> -static FORCEINLINE BOOLEAN
> -SGListNext(
> -    IN OUT  PXENVBD_SG_LIST         SGList,
> -    IN  ULONG                       AlignmentMask
> -    )
> -{
> -    SGList->Length = 0;
> -    SGListGet(SGList);  // get next PhysAddr and PhysLen
> -    return !((SGList->PhysAddr.QuadPart & AlignmentMask) || (SGList-
> >PhysLen & AlignmentMask));
> -}
> -
> -static FORCEINLINE VOID
>  RequestCopyOutput(
>      __in PXENVBD_REQUEST         Request
>      )
> @@ -820,33 +756,39 @@ RequestCopyOutput(
> 
>  static BOOLEAN
>  PrepareSegment(
> -    IN  PXENVBD_TARGET             Target,
> +    IN  PXENVBD_TARGET          Target,
>      IN  PXENVBD_SEGMENT         Segment,
> -    IN  PXENVBD_SG_LIST         SGList,
> +    IN  PXENVBD_SRBEXT          SrbExt,
>      IN  BOOLEAN                 ReadOnly,
>      IN  ULONG                   SectorsLeft,
>      OUT PULONG                  SectorsNow
>      )
>  {
>      PFN_NUMBER      Pfn;
> +    ULONG           Offset;
> +    ULONG           Length;
>      NTSTATUS        Status;
>      PXENVBD_GRANTER Granter = FrontendGetGranter(Target->Frontend);
>      const ULONG     SectorSize = TargetSectorSize(Target);
>      const ULONG     SectorsPerPage = __SectorsPerPage(SectorSize);
> 
> -    if (SGListNext(SGList, SectorSize - 1)) {
> +    Pfn = AdapterGetNextSGEntry(TargetGetAdapter(Target),
> +                                SrbExt,
> +                                0,
> +                                &Offset,
> +                                &Length);
> +    if ((Offset & (SectorSize - 1)) == 0 &&
> +        (Length & (SectorSize - 1)) == 0) {
>          ++Target->SegsGranted;
>          // get first sector, last sector and count
> -        Segment->FirstSector    = (UCHAR)((__Offset(SGList->PhysAddr) +
> SectorSize - 1) / SectorSize);
> +        Segment->FirstSector    = (UCHAR)((Offset + SectorSize - 1) /
> SectorSize);
>          *SectorsNow             = __min(SectorsLeft, SectorsPerPage - 
> Segment-
> >FirstSector);
>          Segment->LastSector     = (UCHAR)(Segment->FirstSector +
> *SectorsNow - 1);
>          Segment->BufferId       = NULL; // granted, ensure its null
>          Segment->Buffer         = NULL; // granted, ensure its null
>          Segment->Length         = 0;    // granted, ensure its 0
> -        Pfn                     = __Phys2Pfn(SGList->PhysAddr);
> 
> -        ASSERT3U((SGList->PhysLen / SectorSize), ==, *SectorsNow);
> -        ASSERT3U((SGList->PhysLen & (SectorSize - 1)), ==, 0);
> +        ASSERT3U((Length / SectorSize), ==, *SectorsNow);
>      } else {
>          PMDL        Mdl;
> 
> @@ -866,15 +808,19 @@ PrepareSegment(
>          Mdl->Process        = NULL;
>          Mdl->MappedSystemVa = NULL;
>          Mdl->StartVa        = NULL;
> -        Mdl->ByteCount      = SGList->PhysLen;
> -        Mdl->ByteOffset     = __Offset(SGList->PhysAddr);
> -        Segment->Pfn[0]     = __Phys2Pfn(SGList->PhysAddr);
> -
> -        if (SGList->PhysLen < *SectorsNow * SectorSize) {
> -            SGListGet(SGList);
> +        Mdl->ByteCount      = Length;
> +        Mdl->ByteOffset     = Offset;
> +        Segment->Pfn[0]     = Pfn;
> +
> +        if (Length < *SectorsNow * SectorSize) {
> +            Pfn = AdapterGetNextSGEntry(TargetGetAdapter(Target),
> +                                        SrbExt,
> +                                        Length,
> +                                        &Offset,
> +                                        &Length);
>              Mdl->Size       += sizeof(PFN_NUMBER);
> -            Mdl->ByteCount  = Mdl->ByteCount + SGList->PhysLen;
> -            Segment->Pfn[1] = __Phys2Pfn(SGList->PhysAddr);
> +            Mdl->ByteCount  = Mdl->ByteCount + Length;
> +            Segment->Pfn[1] = Pfn;
>          }
>  #pragma warning(pop)
> 
> @@ -922,9 +868,9 @@ fail1:
> 
>  static BOOLEAN
>  PrepareBlkifReadWrite(
> -    IN  PXENVBD_TARGET             Target,
> +    IN  PXENVBD_TARGET          Target,
>      IN  PXENVBD_REQUEST         Request,
> -    IN  PXENVBD_SG_LIST         SGList,
> +    IN  PXENVBD_SRBEXT          SrbExt,
>      IN  ULONG                   MaxSegments,
>      IN  ULONG64                 SectorStart,
>      IN  ULONG                   SectorsLeft,
> @@ -956,7 +902,7 @@ PrepareBlkifReadWrite(
> 
>          if (!PrepareSegment(Target,
>                              Segment,
> -                            SGList,
> +                            SrbExt,
>                              ReadOnly,
>                              SectorsLeft,
>                              &SectorsNow))
> @@ -1075,7 +1021,6 @@ PrepareReadWrite(
>      ULONG64         SectorStart = Cdb_LogicalBlock(Srb);
>      ULONG           SectorsLeft = Cdb_TransferBlock(Srb);
>      LIST_ENTRY      List;
> -    XENVBD_SG_LIST  SGList;
>      ULONG           DebugCount;
> 
>      Srb->SrbStatus = SRB_STATUS_PENDING;
> @@ -1083,9 +1028,6 @@ PrepareReadWrite(
>      InitializeListHead(&List);
>      SrbExt->Count = 0;
> 
> -    RtlZeroMemory(&SGList, sizeof(SGList));
> -    SGList.SGList = StorPortGetScatterGatherList(TargetGetAdapter(Target),
> Srb);
> -
>      while (SectorsLeft > 0) {
>          ULONG           MaxSegments;
>          ULONG           SectorsDone = 0;
> @@ -1102,7 +1044,7 @@ PrepareReadWrite(
> 
>          if (!PrepareBlkifReadWrite(Target,
>                                     Request,
> -                                   &SGList,
> +                                   SrbExt,
>                                     MaxSegments,
>                                     SectorStart,
>                                     SectorsLeft,
> --
> 2.8.3
> 
> 
> _______________________________________________
> win-pv-devel mailing list
> win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel
_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel