Comments:
1. Can we add EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST to
mAhciAtaPassThruPpiListTemplate/mAhciBlkIoPpiListTemplate as well? I understand
that requires calling PeiServicesInstallPpi() 3 times instead of once. But I
see the benefit of change is the code is more understandable and potential
bug-free. And you could also create a global mAhciBlkIoPpi/mAhciBlkIo2Ppi
variables and let the mAhciBlkIoPpiListTemplate/mAhciBlkIo2PpiListTemplate
point to them respectively. The reason we cannot have a global AtaPassThruPpi
is there is a Mode field in the PPI that cannot be read-only.
2. Below code snip is used by
AhciBlockIoReadBlocks(),AhciBlockIoGetMediaInfo(), AhciBlockIoGetMediaInfo2()
in AhciPeiBlockIo.c, maybe you can re-write SearchDeviceByIndex() to include
the below code.
Private = GET_AHCI_PEIM_HC_PRIVATE_DATA_FROM_THIS_BLKIO (This);
//
// Check parameters
//
if ((This == NULL) ||
(DeviceIndex == 0) ||
(DeviceIndex > Private->ActiveDevices)) {
return EFI_INVALID_PARAMETER;
}
DeviceData = SearchDeviceByIndex (Private, DeviceIndex);
if (DeviceData == NULL) {
return EFI_NOT_FOUND;
}
3. For the checks in AhciRead(), how about re-order as below?
BlockSize = DeviceData->Media.BlockSize;
if ((BufferSize % BlockSize) != 0) {
return EFI_BAD_BUFFER_SIZE;
}
if (StartLba > DeviceData->Media.LastBlock) {
return EFI_INVALID_PARAMETER;
}
NumberOfBlocks = BufferSize / BlockSize;
if (NumberOfBlocks - 1 > DeviceData->Media.LastBlock - StartLba) {
return EFI_INVALID_PARAMETER;
}
if (BufferSize == 0) {
return EFI_SUCCESS;
}
if (Buffer == NULL) {
return EFI_INVALID_PARAMETER;
}
4. Can you explain more for the first patch? Why VTd PEI module cannot be
changed to support 0xFFFF?
//
// Due to limited resource on VTd PEI DMA buffer size, driver limits the
// maximum transfer block number for 48-bit addressing.
// Setting to 0x800 here means 1M bytes for device with 512-byte block size.
//
#define MAX_48BIT_TRANSFER_BLOCK_NUM 0x800
> -----Original Message-----
> From: Wu, Hao A
> Sent: Tuesday, April 9, 2019 6:13 PM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a...@intel.com>; Ni, Ray <ray...@intel.com>; Dong,
> Eric <eric.d...@intel.com>; Wang, Jian J <jian.j.w...@intel.com>
> Subject: [PATCH v2 0/2] Add PEI BlockIO support for ATA AHCI mode devices
>
> The series is also available at:
> https://github.com/hwu25/edk2/tree/ahci_pei_blockio_v2
>
>
> V2 changges:
>
> Due to the file license change, rebase the whole series onto the tip of the
> master branch. The 'Contributed-under' tag is removed from the log
> messages as well.
>
>
> V1 history:
>
> The series will add the PEI BlockIO support for ATA AHCI mode devices.
>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Jian J Wang <jian.j.w...@intel.com>
>
> Hao Wu (2):
> MdeModulePkg/AhciPei: Limit max transfer blocknum for 48-bit address
> MdeModulePkg/AhciPei: Add PEI BlockIO support
>
> MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf | 4 +
> MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h | 30 ++
> MdeModulePkg/Bus/Ata/AhciPei/AhciPeiBlockIo.h | 257 ++++++++++
> MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c | 125 ++++-
> MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c | 35 +-
> MdeModulePkg/Bus/Ata/AhciPei/AhciPeiBlockIo.c | 526
> ++++++++++++++++++++
> 6 files changed, 975 insertions(+), 2 deletions(-) create mode 100644
> MdeModulePkg/Bus/Ata/AhciPei/AhciPeiBlockIo.h
> create mode 100644 MdeModulePkg/Bus/Ata/AhciPei/AhciPeiBlockIo.c
>
> --
> 2.12.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#39371): https://edk2.groups.io/g/devel/message/39371
Mute This Topic: https://groups.io/mt/31014379/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
