Re: Functionality issues in UDF support

[Paulo Alcantara <pcacjr@zytor.com>]

Hi Ray,

On September 15, 2017 12:33:04 AM GMT-03:00, "Ni, Ruiyu" <ruiyu.ni@intel.com> wrote:
>Paulo,
>Before raising my questions, I'd like to confirm that for a single
>CD/DVD in UDF format, there might be multiple volumes.
>One of the volume could be Eltorito type.
>If my understanding is correct, please continue reading below.

Right.

>
>We found below mapping table using "map -r" shell command in a platform
>with only PartitionDxe change and without UdfDxe driver.
>It's a bug that <BLK6> and <BLK7/FS2> are created. Actually they are
>identical to <BLK3> and <BLK4/FS1>.
>
>--- Mapping table---
>     BLK2: Alias(s):
>          PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)
>
>     BLK3: Alias(s):
>          PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)/CDROM(0x0)
>      FS1: Alias(s):CD1a65535a1:;BLK4:
>          PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)/CDROM(0x1)
>
>     BLK5: Alias(s):
>PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)/VenMedia(C5BD4D42-1A76-4996-8956-73CDA326CD0A)
>     BLK6: Alias(s):
>PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)/VenMedia(C5BD4D42-1A76-4996-8956-73CDA326CD0A)/CDROM(0x0)
>      FS2: Alias(s):CD1a65535ab:;BLK7:
>PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)/VenMedia(C5BD4D42-1A76-4996-8956-73CDA326CD0A)/CDROM(0x1)
>--- End of mapping table ---
>
>After investigation, I found the UDF logic in Partition driver doesn't
>truly skip the Eltorito volume.
>The code flow is like below:
>
>  1.  <BLK2> is created by ScsiDiskDxe driver.
>  2.  By passing <BLK2> to PartitionDxe Start()
>*   <BLK3> and <BLK4/FS1> are created by PartitionDxe driver, by
>PartitionInstallElToritoChildHandles().
>*   <BLK5> is created by PartitionDxe driver, by
>PartitionInstallUdfChildHandles().
>  3.  By passing <BLK5> to PartitionDxe Start()
>*   <BLK6> and <BLK7/FS2> are created by PartitionDxe driver, by
>PartitionInstallElToritoChildHandles().
>
>I think step 2.a is not correct if my understanding to UDF is correct.
>The PartitionInstallUdfChildHandles() should iterate
>all volumes in the media and creates the child BLK handle for each
>volume, but skipping Eltorito volume.
>
>Instead, the current implementation just creates one child BLK handle
>for the entire media.
>To avoid reclusively creating  child BLK handle, the
>PartitionInstallUdfChildHandles() contains a logic to do nothing when
>the handle is created by ParititonDxe driver. The logic is not needed
>when the implementation follows my suggestion above.
>Due to this, step 3.a creates the additional but shouldn't-exist BLK
>handles <BLK6> and <BLK7/FS2>.
>
>UdfDxe driver is supposed to Start() on each volume and produce
>SimpleFileSystem protocol.

It seems that PartitionInstallUdfChildHandles() indeed skips the ElTorito partitions otherwise you'd see the ".../CDROM(0x1)/VenMedia()" and ".../CDROM(0x0)/VenMedia()" device paths *also* in the mapping output.

If I understand correctly, the problem is that when we create a child handle for an UDF volume, Partition driver will execute again, parse the newly created UDF handle, find again an ElTorito partition and then install a new child handle (VenMedia()/CDROM())

So, the logic of skipping of ElTorito partitions in PartitionInstallUdfChildHandles() is not enough. Unfortunately we can't handle UDF bridge disks (ElTorito + UDF) entirely in PartitionInstallUdfChildHandles() -- we should probably also skip UDF device paths in PartitionInstallElToritoChildHandles().

Does it make sense to you, Ray? Thanks for raising this up.

Paulo

>
>Do you agree with my above suggestions?
>
>Laszlo,
>I understood your needs of this UDF support. But as you can see there
>are many build failures and even functionality issues due to this
>support.
>I am not sure how the other open source project handles such cases.
>But I am thinking maybe we could move the whole UDF support to
>edk2-staging firstly and move it back after all the issues are
>resolved.
>What's your suggestion?
>
>Thanks/Ray

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.