Re: [ros-svn] [hbirr] 17665: - Copy the map registers to the buffer only, if they are used (in IoFlushAdapterBuffers).
5 Sep
2005
5 Sep
'05
8:43 p.m.
hbirr(a)svn.reactos.com wrote:
- Copy the map registers to the buffer only, if they
are used (in IoFlushAdapterBuffers).
- Do not use the byte offset into the page from a given buffer if the map registers are
used,
because the caller didn't request for one additional register in the call to
IoAllocateAdapterChannel
and it will not work for a 64k buffer.
Sorry, but these changes are wrong. I won't argue that there isn't bug,
but you're just workarounding it.
- At first, the UseMapRegisters variable you added to MAP_REGISTER_ENTRY
structure isn't needed. IoFlushAdapterBuffers can determine the
information itself and the case was clearly marked as UNIMPLEMENTED and
FIXME.
- The byte offset is added for a reason. I know the stuff that it is
supposed to solve is broken atm, but the rationale is that there can be
more IoMapTransfer calls and together with MAP_REGISTER_ENTRY->Counter
you can setup more transfers... It's explained in
http://download.microsoft.com/download/e/b/a/eba1050f-a31d-436b-9281-92cdfe….
I certainly welcome any help and review of the DMA code and I'll try to
add comments where appropriate, but I would like to discuss these
changes first on the Mailing List / IRC / mail since this piece of code
is pretty complex and any changes are prone to break some feature...
- Filip
5 Sep
5 Sep
10:06 p.m.
New subject: [ros-svn] [hbirr] 17665: - Copy the map registers to the buffer only, if they are used (in IoFlushAdapterBuffers).
Filip Navara wrote:
- At first, the UseMapRegisters variable you added to
MAP_REGISTER_ENTRY structure isn't needed. IoFlushAdapterBuffers can
determine the information itself and the case was clearly marked as
UNIMPLEMENTED and FIXME.
This case isn't marked as UNIMPLEMENTED and FIXME. The using of hardware
scatter/gather is marked as UNIMPLEMENTED and FIXME. I've modified the
cache manager a little bit. The cache segments for the floppy driver
allocated from below the 16MB limit. I get no data from a floppy.
- The byte offset is added for a reason. I know the
stuff that it is
supposed to solve is broken atm, but the rationale is that there can
be more IoMapTransfer calls and together with
MAP_REGISTER_ENTRY->Counter you can setup more transfers... It's
explained in
http://download.microsoft.com/download/e/b/a/eba1050f-a31d-436b-9281-92cdfe…
It makes no sense to call IoMapTransfer more than ones without a call to
IoFlushAdapterBuffers for the pc dma controllers. For the pc dma
controllers, a transfer has to be finished, before the next can be
started. It is always possible to call IoFlushAdapterBuffers after each
transfer and before the next is started. The reference doesn't explain
something about to setup more transfers.
- Hartmut
10:50 p.m.
New subject: [ros-svn] [hbirr] 17665: - Copy the map registers to the buffer only, if they are used (in IoFlushAdapterBuffers).
Hartmut Birr wrote:
It applies to non-system-DMA... Quoting NT4 DDK documentation: "The
driver of a busmaster device with scatter/gather support can use the
returned logical address and updated Length value to build a
scatter/gather list, calling IoMapTransfer repeatedly until it has used
all available map registers for the transfer operation."
Filip Navara wrote:
The UNIMPLEMENTED case is entered for any adapter that claims that it
can support hardware scatter/gather regardless of it being used or not.
A quick untested patch is attached (I'm not sure it will work, since it
looks like I forgot to initialize MapRegisterBase->Counter...). I'm
planning to look at it.
- At first, the UseMapRegisters variable you added
to
MAP_REGISTER_ENTRY structure isn't needed. IoFlushAdapterBuffers can
determine the information itself and the case was clearly marked as
UNIMPLEMENTED and FIXME.
This case isn't marked as UNIMPLEMENTED and FIXME. The using of hardware
scatter/gather is marked as UNIMPLEMENTED and FIXME. I've modified the
cache manager a little bit. The cache segments for the floppy driver
allocated from below the 16MB limit. I get no data from a floppy.
- The byte
offset is added for a reason. I know the stuff that it is
supposed to solve is broken atm, but the rationale is that there can
be more IoMapTransfer calls and together with
MAP_REGISTER_ENTRY->Counter you can setup more transfers... It's
explained in
http://download.microsoft.com/download/e/b/a/eba1050f-a31d-436b-9281-92cdfe…
It makes no sense to call IoMapTransfer more than ones without a call to
IoFlushAdapterBuffers for the pc dma controllers. For the pc dma
controllers, a transfer has to be finished, before the next can be
started.
It is always possible to call IoFlushAdapterBuffers
after each
transfer and before the next is started. The reference doesn't explain
something about to setup more transfers.
See above.
I'll try to write more tomorrow, but my mind is getting foggy now...
Best regards,
Filip
11:07 p.m.
New subject: [ros-svn] [hbirr] 17665: - Copy the map registers to the buffer only, if they are used (in IoFlushAdapterBuffers).
Filip Navara wrote:
The
UNIMPLEMENTED case is entered for any adapter that claims that it
can support hardware scatter/gather regardless of it being used or
not. A quick untested patch is attached (I'm not sure it will work,
since it looks like I forgot to initialize
MapRegisterBase->Counter...). I'm planning to look at it.
The patch didn't work, because MAP_BASE_SW_SG is always set. The new
code in IoFlushAdapterBuffers isn't executed for the floppy dma.
- Hartmut
11:13 p.m.
New subject: [ros-svn] [hbirr] 17665: - Copy the map registers to the buffer only, if they are used (in IoFlushAdapterBuffers).
Filip Navara wrote:
Hartmut Birr wrote:
It applies to non-system-DMA... Quoting NT4 DDK documentation: "The
driver of a busmaster device with scatter/gather support can use the
returned logical address and updated Length value to build a
scatter/gather list, calling IoMapTransfer repeatedly until it has
used all available map registers for the transfer operation."
Oh, this time with the right patch...
- Filip
Filip Navara wrote:
The UNIMPLEMENTED case is entered for any adapter that claims that it
can support hardware scatter/gather regardless of it being used or
not. A quick untested patch is attached (I'm not sure it will work,
since it looks like I forgot to initialize
MapRegisterBase->Counter...). I'm planning to look at it.
- At first, the UseMapRegisters variable you
added to
MAP_REGISTER_ENTRY structure isn't needed. IoFlushAdapterBuffers can
determine the information itself and the case was clearly marked as
UNIMPLEMENTED and FIXME.
This case isn't marked as UNIMPLEMENTED and FIXME. The using of hardware
scatter/gather is marked as UNIMPLEMENTED and FIXME. I've modified the
cache manager a little bit. The cache segments for the floppy driver
allocated from below the 16MB limit. I get no data from a floppy.
- The
byte offset is added for a reason. I know the stuff that it is
supposed to solve is broken atm, but the rationale is that there can
be more IoMapTransfer calls and together with
MAP_REGISTER_ENTRY->Counter you can setup more transfers... It's
explained in
http://download.microsoft.com/download/e/b/a/eba1050f-a31d-436b-9281-92cdfe…
It makes no sense to call IoMapTransfer more than ones without a call to
IoFlushAdapterBuffers for the pc dma controllers. For the pc dma
controllers, a transfer has to be finished, before the next can be
started.
It is always possible to call
IoFlushAdapterBuffers after each
transfer and before the next is started. The reference doesn't explain
something about to setup more transfers.
See above.
I'll try to write more tomorrow, but my mind is getting foggy now...
6 Sep
6 Sep
8:11 p.m.
New subject: [ros-svn] [hbirr] 17665: - Copy the map registers to the buffer only, if they are used (in IoFlushAdapterBuffers).
Filip Navara wrote:
Oh, this time with the right patch...
The patch works for me with a little modification. One test condition
must be changed in HalpCopyBufferMap.
- Hartmut
Index: hal/halx86/generic/dma.c
===================================================================
--- hal/halx86/generic/dma.c (Revision 17704)
+++ hal/halx86/generic/dma.c (Arbeitskopie)
@@ -1556,6 +1556,9 @@
{
BOOLEAN SlaveDma = FALSE;
PMAP_REGISTER_ENTRY RealMapRegisterBase;
+ PHYSICAL_ADDRESS HighestAcceptableAddress;
+ PHYSICAL_ADDRESS PhysicalAddress;
+ PPFN_NUMBER MdlPagesPtr;
ASSERT_IRQL(DISPATCH_LEVEL);
@@ -1588,18 +1591,27 @@
{
if ((ULONG_PTR)MapRegisterBase & MAP_BASE_SW_SG)
{
- if (RealMapRegisterBase->Counter != ~0)
+ if (RealMapRegisterBase->Counter == ~0)
{
if (SlaveDma && !AdapterObject->IgnoreCount)
Length -= HalReadDmaCounter(AdapterObject);
+ HalpCopyBufferMap(Mdl, RealMapRegisterBase, CurrentVa, Length, FALSE);
}
-
- HalpCopyBufferMap(Mdl, RealMapRegisterBase, CurrentVa, Length, FALSE);
}
else
{
- /* FIXME: Unimplemented case */
- ASSERT(FALSE);
+ MdlPagesPtr = MmGetMdlPfnArray(Mdl);
+ MdlPagesPtr += ((ULONG_PTR)CurrentVa - (ULONG_PTR)Mdl->StartVa) >>
PAGE_SHIFT;
+
+ PhysicalAddress.QuadPart = *MdlPagesPtr << PAGE_SHIFT;
+ PhysicalAddress.QuadPart += BYTE_OFFSET(CurrentVa);
+
+ HighestAcceptableAddress = HalpGetAdapterMaximumPhysicalAddress(AdapterObject);
+ if (PhysicalAddress.QuadPart + Length >
+ HighestAcceptableAddress.QuadPart)
+ {
+ HalpCopyBufferMap(Mdl, RealMapRegisterBase, CurrentVa, Length, FALSE);
+ }
}
}
@@ -1729,7 +1741,6 @@
* pages that are physically contiguous and that don't cross the
* 64Kb boundary (this limitation applies only for ISA controllers).
*/
-
while (TransferLength < *Length)
{
MdlPage1 = *MdlPagesPtr;
@@ -1784,11 +1795,12 @@
HighestAcceptableAddress.QuadPart)
{
UseMapRegisters = TRUE;
- PhysicalAddress = RealMapRegisterBase->PhysicalAddress;
+ PhysicalAddress = RealMapRegisterBase[Counter].PhysicalAddress;
PhysicalAddress.QuadPart += ByteOffset;
if ((ULONG_PTR)MapRegisterBase & MAP_BASE_SW_SG)
{
RealMapRegisterBase->Counter = ~0;
+ Counter = 0;
}
}
}
7051
days inactive
7052
days old
5 comments
2 participants
participants (2)
-
Filip Navara -
Hartmut Birr