Re: [edk2] [RFC PATCH] Add VirtIO disks to legacy bootable options

Subject: Re: [edk2] [RFC PATCH] Add VirtIO disks to legacy bootable options

From: Laszlo Ersek <lersek@redhat.com>

To: David Woodhouse <dwmw2@infradead.org>, Jordan Justen <jljusten@gmail.com>

Date: 2013-01-28 17:26:44

On 01/25/13 14:53, David Woodhouse wrote:

> In fact we probably also want to be looking for the DiskInfoProtocol
> (and implementing that in VirtioBlkDxe, getting the Identify data for
> the drive and using its *name*.

What do we need the name for?

EFI_DISK_INFO_PROTOCOL provides a data field called "Interface" (a GUID)
that identifies the data format for the Inquiry(), Identify(), and
SenseData() functions that it also provides. The fourth function it
makes available is WhichIde().

4 interface GUIDs appear to be defined:

guid                               Inquiry                Identify             SenseData            WhichIde
---------------------------------  ---------------------  -------------------  -------------------  ---------------
EFI_DISK_INFO_IDE_INTERFACE_GUID   unsupported            ATA_IDENTIFY_DATA    unsupported          port/multiplier
EFI_DISK_INFO_SCSI_INTERFACE_GUID  EFI_SCSI_INQUIRY_DATA  ATAPI_IDENTIFY_DATA  EFI_SCSI_SENSE_DATA  channel/dev
EFI_DISK_INFO_USB_INTERFACE_GUID   USB_BOOT_INQUIRY_DATA  unsupported          unsupported          unsupported
EFI_DISK_INFO_AHCI_INTERFACE_GUID  unimplemented          unimplemented        unimplemented        unimplemented

Without looking at the guid first, it's not possible to parse the
Inquiry/Identify/SenseData buffers.

ATA_IDENTIFY_DATA provides ModelName.

So does ATAPI_IDENTIFY_DATA, but ATAPI_IDENTIFY_DATA / Identify()
themselves are not supported on a "Physical SCSI bus" (see
ScsiDiskInfoIdentify() in
"MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c").

USB mass storage doesn't support Identify().

For AHCI there's no actual EFI_DISK_INFO_PROTOCOL implementation in
TianoCore.

For virtio-blk:

- I would report no support for Inquiry/SenseData/WhichIde (the spec
allows this),

- I would prefer to introduce a new GUID, and a corresponding new buffer
type for Identify: VIRTIO_BLK_IDENTIFY_DATA, containing nothing else
than a constant "model name".

Reusing EFI_DISK_INFO_IDE_INTERFACE_GUID is a bad idea, as callers would
rightfully expect

- WhichIde() to work (makes no sense for virtio-blk),
- the rest of ATA_IDENTIFY_DATA to be filled in (ditto).

IOW there's no "GetModelName()" call that would blindly work with any
EFI_DISK_INFO_PROTOCOL instance; the client must look at both the
Identify() return value and the GUID before attempting to parse the
model name. Since the client code would contain an explicit comparison
against "EFI_DISK_INFO_VBLK_INTERFACE_GUID", I'm not sure we'd be better
off this way than by checking the driver name.

What are your thoughts?

Thanks,
Laszlo

------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnnow-d2d
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel