public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ni, Ruiyu" <ruiyu.ni@intel.com>
To: Paulo Alcantara <pcacjr@zytor.com>, Laszlo Ersek <lersek@redhat.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"Wu, Hao A" <hao.a.wu@intel.com>,
	"Zeng, Star" <star.zeng@intel.com>
Subject: Functionality issues in UDF support
Date: Fri, 15 Sep 2017 03:33:04 +0000	[thread overview]
Message-ID: <734D49CCEBEEF84792F5B80ED585239D5BA4206B@SHSMSX151.ccr.corp.intel.com> (raw)

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.

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.

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



             reply	other threads:[~2017-09-15  3:31 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-15  3:33 Ni, Ruiyu [this message]
2017-09-15  4:57 ` Functionality issues in UDF support Paulo Alcantara
2017-09-15  5:08   ` Ni, Ruiyu
2017-09-15  5:33     ` Zeng, Star
2017-09-15  5:35       ` Ni, Ruiyu
2017-09-15  5:37     ` Paulo Alcantara
2017-09-15  6:02       ` Ni, Ruiyu
2017-09-15  6:26         ` Paulo Alcantara
2017-09-15  7:01           ` Zeng, Star
2017-09-15  7:08             ` Ni, Ruiyu
2017-09-15  7:10           ` Ni, Ruiyu
2017-09-15  7:22             ` Paulo Alcantara
2017-09-15  7:38               ` Ni, Ruiyu
2017-09-15 10:26                 ` Laszlo Ersek
2017-09-15 11:03                 ` Laszlo Ersek
2017-09-15 12:45                   ` Ni, Ruiyu
2017-09-15 15:51                 ` Laszlo Ersek
2017-09-15 16:27                   ` Paulo Alcantara
2017-09-15 16:40                     ` Ni, Ruiyu
2017-09-15 16:51                       ` Laszlo Ersek
2017-09-15 23:38                         ` Yao, Jiewen
2017-09-15 23:58                           ` Yao, Jiewen
2017-09-16 21:19                             ` Laszlo Ersek
2017-09-16 20:21                           ` Laszlo Ersek
2017-09-15 16:45                     ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=734D49CCEBEEF84792F5B80ED585239D5BA4206B@SHSMSX151.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox