On 28 August 2016 at 00:46, Alan Ott alan@softiron.co.uk wrote:
On 08/27/2016 11:20 AM, Ard Biesheuvel wrote:
On 26 August 2016 at 23:57, Alan Ott alan@softiron.co.uk wrote:
The Dual Address Cycle Attribute is necessary for bus mastering PCI devices which are capable of generating 64-bit addresses. See the "Driver Writer's Guide for UEFI 2.3.1" version 1.01, section 18.3.2.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Alan Ott alan@softiron.co.uk
Drivers/Net/MarvellYukonDxe/if_msk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Drivers/Net/MarvellYukonDxe/if_msk.c b/Drivers/Net/MarvellYukonDxe/if_msk.c index 8820f2a..4ca3c5f 100644 --- a/Drivers/Net/MarvellYukonDxe/if_msk.c +++ b/Drivers/Net/MarvellYukonDxe/if_msk.c @@ -1160,7 +1160,7 @@ mskc_attach ( Status = mPciIo->Attributes ( mPciIo, EfiPciIoAttributeOperationEnable,
Supports,
Supports | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE, NULL );Interestingly, although this pattern occurs more often in EDK2 code, it actually violated the spec, since it states
""" EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLEï€ This bit may only be used in the Attributes parameter to AllocateBuffer(). If this bit is set, then the PCI controller that is requesting a buffer through AllocateBuffer() is capable of producing PCI Dual Address Cycles, so it is able to access a 64-bit address space. If this bit is not set, then the PCI controller that is requesting a buffer through AllocateBuffer() is not capable of producing PCI Dual Address Cycles, so it is only able to access a 32-bit address space. """
Like the EfiPciOperationBusMasterWrite64 attribute I spoke of in the other email, EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE is only an attribute for the PCI Root Bridge version of AllocateBuffer(), not for the PCI IO version of AllocateBuffer(). See the 2.6 UEFI Spec, sections 13.2 vs 13.4.
Ah yes, thanks for clearing that up.
This is out of sync with the UEFI driver writer's guide, which suggests what you are doing here.
We should get this clarified, but in the mean time, this looks correct to me.
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Thanks for reviewing. It is appreciated.
Alan.