Thanks.
Reviewed-by: Ruiyu Ni ruiyu...@intel.com<mailto:ruiyu...@intel.com>
From: Olivier Martin [mailto:olivier.mar...@arm.com]
Sent: Wednesday, January 7, 2015 12:02 AM
To: Ni, Ruiyu
Cc: edk2-devel@lists.sourceforge.net
Subject: RE: [edk2] [PATCH] MdeModulePkg/PartitionDxe: Fixed El Torito support
when the medium is not a CDROM
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
<olivier.mar...@arm.com<mailto:olivier.mar...@arm.com>>
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: 23 December 2014 01:23
To: Olivier Martin
Cc: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
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:olivier.mar...@arm.com]
Sent: Monday, December 22, 2014 11:21 PM
To: Ni, Ruiyu
Cc: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
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
<olivier.mar...@arm.com<mailto:olivier.mar...@arm.com>>
Thanks,
Olivier
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: 10 December 2014 06:19
To: Olivier Martin
Cc: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
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:ruiyu...@intel.com]
Sent: Monday, November 17, 2014 11:07 AM
To: Olivier Martin
Cc: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
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:olivier.mar...@arm.com]
Sent: Tuesday, October 21, 2014 1:09 AM
To: Tian, Feng
Cc: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
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
<olivier.mar...@arm.com<mailto:olivier.mar...@arm.com>>
Regards,
Olivier
------------------------------------------------------------------------------
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
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
