On 12/17/14 17:24, Olivier Martin wrote: > Ok, I missed VIRTIO_TRANSPORT_DEVICE_PATH definition in VirtFdtDxe. > Should this definition be moved into > OvmfPkg/Include/Guid/VirtioMmioTransport.h? > I checked if there were some protocols that were defining their special > device path. And I found > MdePkg/Include/Protocol/DebugPort.h > > If we do not move the definition there, I would need to either duplicate the > definition or create a new one for > ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmFvpDxe.c.
When ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmFvpDxe.c was first under review here, I think I mentioned a theoretical need for separate device paths (for separate handles). However, this is not a problem for "ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmFvpDxe.c", because: - you only define one virtio-mmio transport (because the FVP / Foundation models support only one, at a fixed address) - you don't need that device path to match up with anything else - the GUID you use there is EFI_CALLER_ID_GUID (ie. comes from the INF file), which is different from everything else, so it allows you to construct a completely custom device path node If you'd like to "conform" to the same VIRTIO_TRANSPORT_DEVICE_PATH in ArmFvpDxe.c, I can move the struct to "OvmfPkg/Include/Guid/VirtioMmioTransport.h". But, at the moment there is certainly no requirement for ArmFvpDxe.c to follow that pattern. For "complete" integration across all possible clients, the device path formatting and installation could be pushed down even to function VirtioMmioInstallDevice(), in "OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.c". Then it wouldn't just install gVirtioDeviceProtocolGuid on the handle, but also the device path protocol. (And the lifecycle of Handle would have to be updated slightly, because the first protocol installation creates the handle itself.) In this patchset I have operated with the idea (== current fact) that each driver that installs a virtio-mmio device is free to pick its own device path format for it. If we'd like to tighten that, it's doable, just more work. Thanks Laszlo > >> -----Original Message----- >> From: Laszlo Ersek [mailto:[email protected]] >> Sent: 16 December 2014 19:38 >> To: [email protected]; [email protected]; >> [email protected]; [email protected] >> Subject: Re: [edk2] [PATCH v3 07/13] OvmfPkg: introduce >> VIRTIO_MMIO_TRANSPORT_GUID >> >> On 12/16/14 15:35, Olivier Martin wrote: >>> Something I do not really used but maybe I should is the 'Data' part >> of >>> 'VenHw(Guid, Data)'. >>> The reason I am saying that is the Device Path you are defining would >> only >>> be able to define a single virtio-mmio transport bus. >> >> The VenHw() device node in the device path that VirtFdtDxe associates >> with the handle that carries the virtio protocol instance already does >> exactly this. >> >> Please see the handling of PropertyTypeVirtio in function >> InitializeVirtFdtDxe(), file >> "ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c" -- we >> iterate over all virtio-mmio transports named in the DTB, and the >> VenHw() node for each -- carrying the same GUID -- encodes the MMIO >> base >> address of the register block in the Data portion. >> >> #pragma pack (1) >> typedef struct { >> VENDOR_DEVICE_PATH Vendor; >> UINT64 PhysBase; >> EFI_DEVICE_PATH_PROTOCOL End; >> } VIRTIO_TRANSPORT_DEVICE_PATH; >> #pragma pack () >> >> ... >> >> DevicePath->PhysBase = RegBase; >> >> In fact that data portion is *precisely* the thing that the boot order >> matching relies on. >> >>> So it means we should not necessary return if we fail to install the >>> virtio-mmio transport device path. >> >> I don't understand this. >> >> Hm, perhaps you mean that UEFI doesn't allow you to install the same >> device path on two different handles (because that would make it >> impossible to uniquely locate a handle by device path). And, this case >> would be triggered if we didn't distinguish the VenHw() nodes from each >> other that we associate with the different virtio transports. >> >> This train of thought is correct, but we *do* differentiate the device >> paths from each other, by encoding the virtio-mmio register block base >> address in the Data portion of the VenHw() nodes. >> >>> We should probably retrieve the handle of >>> the owner of the virtio-mmio transport device path to install the >> protocols >>> we want to support. >>> >>> Another way to define the virtio-mmio transport device path node >> would be: >>> VenHw(gVirtioMmioTransportGuid, Guid) >> >> This patch (and the next patch) preserve the behavior I described >> above; >> they just give the fixed GUID a "public" name. >> >> Thanks >> Laszlo >> >>> >>>> -----Original Message----- >>>> From: Laszlo Ersek [mailto:[email protected]] >>>> Sent: 11 December 2014 02:46 >>>> To: [email protected]; [email protected]; >>>> [email protected]; [email protected] >>>> Subject: [edk2] [PATCH v3 07/13] OvmfPkg: introduce >>>> VIRTIO_MMIO_TRANSPORT_GUID >>>> >>>> Soon there will be more than one modules (in separate packages) that >>>> need >>>> to have an understanding about the GUID used in the VenHw() device >> path >>>> nodes that describe virtio-mmio transports. Define such a GUID >>>> explicitly. >>>> >>>> Preserve the current value (which happens to be the FILE_GUID of >>>> ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf) for >>>> compatibility with external users. >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>> Signed-off-by: Laszlo Ersek <[email protected]> >>>> --- >>>> OvmfPkg/Include/Guid/VirtioMmioTransport.h | 25 >>>> +++++++++++++++++++++++++ >>>> OvmfPkg/OvmfPkg.dec | 1 + >>>> 2 files changed, 26 insertions(+) >>>> >>>> diff --git a/OvmfPkg/Include/Guid/VirtioMmioTransport.h >>>> b/OvmfPkg/Include/Guid/VirtioMmioTransport.h >>>> new file mode 100644 >>>> index 0000000..f17d7b1 >>>> --- /dev/null >>>> +++ b/OvmfPkg/Include/Guid/VirtioMmioTransport.h >>>> @@ -0,0 +1,25 @@ >>>> +/** @file >>>> + Recommended GUID to be used in the Vendor Hardware device path >> nodes >>>> that >>>> + identify virtio-mmio transports. >>>> + >>>> + Copyright (C) 2014, Red Hat, Inc. >>>> + >>>> + This program and the accompanying materials are licensed and made >>>> available >>>> + under the terms and conditions of the BSD License that >> accompanies >>>> this >>>> + distribution. The full text of the license may be found at >>>> + http://opensource.org/licenses/bsd-license.php. >>>> + >>>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >>>> BASIS, WITHOUT >>>> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR >>>> IMPLIED. >>>> + >>>> +**/ >>>> + >>>> +#ifndef __VIRTIO_MMIO_TRANSPORT_H__ >>>> +#define __VIRTIO_MMIO_TRANSPORT_H__ >>>> + >>>> +#define VIRTIO_MMIO_TRANSPORT_GUID \ >>>> +{0x837dca9e, 0xe874, 0x4d82, {0xb2, 0x9a, 0x23, 0xfe, 0x0e, 0x23, >>>> 0xd1, 0xe2}} >>>> + >>>> +extern EFI_GUID gVirtioMmioTransportGuid; >>>> + >>>> +#endif >>>> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec >>>> index 8802392..41db92d 100644 >>>> --- a/OvmfPkg/OvmfPkg.dec >>>> +++ b/OvmfPkg/OvmfPkg.dec >>>> @@ -52,6 +52,7 @@ >>>> gUefiOvmfPkgTokenSpaceGuid = {0x93bb96af, 0xb9f2, 0x4eb8, >>>> {0x94, 0x62, 0xe0, 0xba, 0x74, 0x56, 0x42, 0x36}} >>>> gEfiXenInfoGuid = {0xd3b46f3b, 0xd441, 0x1244, >>>> {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d}} >>>> gOvmfPlatformConfigGuid = {0x7235c51c, 0x0c80, 0x4cab, >>>> {0x87, 0xac, 0x3b, 0x08, 0x4a, 0x63, 0x04, 0xb1}} >>>> + gVirtioMmioTransportGuid = {0x837dca9e, 0xe874, 0x4d82, >>>> {0xb2, 0x9a, 0x23, 0xfe, 0x0e, 0x23, 0xd1, 0xe2}} >>>> >>>> [Protocols] >>>> gVirtioDeviceProtocolGuid = {0xfa920010, 0x6785, 0x4941, >>>> {0xb6, 0xec, 0x49, 0x8c, 0x57, 0x9f, 0x16, 0x0a}} >>>> -- >>>> 1.8.3.1 >>>> >>>> >>>> >>>> -------------------------------------------------------------------- >> --- >>>> ------- >>>> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server >>>> from Actuate! Instantly Supercharge Your Business Reports and >>>> Dashboards >>>> with Interactivity, Sharing, Native Excel Exports, App Integration & >>>> more >>>> Get technology previously reserved for billion-dollar corporations, >>>> FREE >>>> >> http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.c >>>> lktrk >>>> _______________________________________________ >>>> edk2-devel mailing list >>>> [email protected] >>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel >>> >>> >>> >>> >>> >>> --------------------------------------------------------------------- >> --------- >>> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server >>> from Actuate! Instantly Supercharge Your Business Reports and >> Dashboards >>> with Interactivity, Sharing, Native Excel Exports, App Integration & >> more >>> Get technology previously reserved for billion-dollar corporations, >> FREE >>> >> http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.c >> lktrk >>> _______________________________________________ >>> edk2-devel mailing list >>> [email protected] >>> https://lists.sourceforge.net/lists/listinfo/edk2-devel >>> >> >> >> ----------------------------------------------------------------------- >> ------- >> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server >> from Actuate! Instantly Supercharge Your Business Reports and >> Dashboards >> with Interactivity, Sharing, Native Excel Exports, App Integration & >> more >> Get technology previously reserved for billion-dollar corporations, >> FREE >> http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.c >> lktrk >> _______________________________________________ >> edk2-devel mailing list >> [email protected] >> https://lists.sourceforge.net/lists/listinfo/edk2-devel > > > > > > ------------------------------------------------------------------------------ > Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server > from Actuate! Instantly Supercharge Your Business Reports and Dashboards > with Interactivity, Sharing, Native Excel Exports, App Integration & more > Get technology previously reserved for billion-dollar corporations, FREE > http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/edk2-devel > ------------------------------------------------------------------------------ Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
