Thanks Ray, good catch!
Please find the attached patch that takes in account your comment. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Olivier Martin <[email protected]> From: Ni, Ruiyu [mailto:[email protected]] Sent: 23 December 2014 01:23 To: Olivier Martin Cc: [email protected] Subject: RE: [edk2] [PATCH] MdeModulePkg/PartitionDxe: Fixed El Torito support when the medium is not a CDROM Olivier, Thank you for the updated patch. If you replace the below while-loop with a for-loop, maybe the IsFirstVolumeDescriptor can be removed to achieve a cleaner code. Is my understanding correct? + VolDescriptorOffset = SIZE_32KB; + // // Loop: handle one volume descriptor per time // + IsFirstVolumeDescriptor = TRUE; while (TRUE) { + if (IsFirstVolumeDescriptor) { + // The volume descriptor offset is already pointed to the correct offset. + // We do not need to increment it. + IsFirstVolumeDescriptor = FALSE; + } else { + // Move to the next volume descriptor + VolDescriptorOffset += SIZE_2KB; + } ---> for (VolDescriptorOffset = SIZE_32KB; ; VolDescriptorOffset += SIZE_2KB) { . } Thanks, Ray From: Olivier Martin [mailto:[email protected]] Sent: Monday, December 22, 2014 11:21 PM To: Ni, Ruiyu Cc: [email protected] Subject: RE: [edk2] [PATCH] MdeModulePkg/PartitionDxe: Fixed El Torito support when the medium is not a CDROM Hi Ray, This attached patch should address all your comments. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Olivier Martin <[email protected]> Thanks, Olivier From: Ni, Ruiyu [mailto:[email protected]] Sent: 10 December 2014 06:19 To: Olivier Martin Cc: [email protected] Subject: RE: [edk2] [PATCH] MdeModulePkg/PartitionDxe: Fixed El Torito support when the medium is not a CDROM Olivier, Any comments? Thanks, Ray From: Ni, Ruiyu [mailto:[email protected]] Sent: Monday, November 17, 2014 11:07 AM To: Olivier Martin Cc: [email protected] Subject: Re: [edk2] [PATCH] MdeModulePkg/PartitionDxe: Fixed El Torito support when the medium is not a CDROM Olivier, I have several comments regarding your patch: 1. if (((2048 % Media->BlockSize) != 0) && (Media->BlockSize > 2048)) { return EFI_NOT_FOUND; } The above check cannot catch the case when Media->BlockSize is 13. So I am wondering maybe "&&" should be "||". Please confirm. 2. IsFirstVolumeDescriptor = TRUE; while (TRUE) { if (IsFirstVolumeDescriptor) { // Move to the next volume descriptor VolDescriptorOffset += SIZE_2KB; IsFirstVolumeDescriptor = FALSE; } I don't understand why it only adds SIZE_2KB in the first loop? Can you explain more? 3. VolDescriptorOffset is type of UINT32. I suggest to change to UINT64. 4. There are two places using 2048. Suggest to change to SIZE_2KB. Thanks, Ray From: Olivier Martin [mailto:[email protected]] Sent: Tuesday, October 21, 2014 1:09 AM To: Tian, Feng Cc: [email protected] Subject: [edk2] [PATCH] MdeModulePkg/PartitionDxe: Fixed El Torito support when the medium is not a CDROM Dear MdeModulePkg maintainer, Please find the attached patch that fixes El Torito format on media that do not have a 2KB block size. El Torito format can be used on different media (eg: USB). A ISO image can be dumped onto a USB mass-storage. These media might not have the same block size as the CDROM media (ie: 2KB). The El Torito code and the the El Torito catalogue assume a 2KB-LBA. In addition, the UEFI specification says in "12.3.4.4 CD-ROM and DVD-ROM": UEFI code does not assume a fixed block size. I was able to duplicate the issue by copying a debian ISO on a USB driver and try to open the device with UEFI. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Olivier Martin <[email protected]> Regards, Olivier
MdeModulePkg-PartitionDxe-Fixed-El-Torito-support.patch
Description: Binary data
------------------------------------------------------------------------------ Dive into the World of Parallel Programming! The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net
_______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
