Hi Abner,
For #4, To see below code. RNDIS driver provide the RndisUndiStart routine.
It could execute RndisUndiStart in UndiStart routine end.
The upper layer driver not need to know the USB CDC driver
interface class.
It only provide the USB_ETHERNET_PROTOCOL. The UNDI protocol is
easy to control by NetworkCommon driver.
I have convert the USB_ETHERNET_PROTOCOL to EDKII_USB_ETHERNET_PROTOCOL, will
send the patch next.
UsbRndisDevice->Signature = USB_RNDIS_SIGNATURE;
UsbRndisDevice->NumOfInterface =
Interface.InterfaceNumber;
UsbRndisDevice->UsbRndisHandle = RndisHandle;
UsbRndisDevice->UsbCdcDataHandle = 0;
UsbRndisDevice->UsbIo = UsbIo;
// ------ Below is for USB_ETHERNET_PROTOCOL
UsbRndisDevice->UsbEth.UsbEthReceive = RndisUndiReceive;
UsbRndisDevice->UsbEth.UsbEthTransmit = RndisUndiTransmit;
UsbRndisDevice->UsbEth.UsbEthInterrupt = UsbRndisInterrupt;
UsbRndisDevice->UsbEth.UsbEthMacAddress = GetUsbEthMacAddress;
UsbRndisDevice->UsbEth.UsbEthMaxBulkSize = UsbEthBulkSize;
UsbRndisDevice->UsbEth.UsbHeaderFunDescriptor =
GetUsbHeaderFunDescriptor;
UsbRndisDevice->UsbEth.UsbUnionFunDescriptor = GetUsbUnionFunDescriptor;
UsbRndisDevice->UsbEth.UsbEthFunDescriptor = GetUsbRndisFunDescriptor;
UsbRndisDevice->UsbEth.SetUsbEthMcastFilter = SetUsbRndisMcastFilter;
UsbRndisDevice->UsbEth.SetUsbEthPowerPatternFilter = SetUsbRndisPowerFilter;
UsbRndisDevice->UsbEth.GetUsbEthPoewrPatternFilter = GetUsbRndisPowerFilter;
UsbRndisDevice->UsbEth.SetUsbEthPacketFilter = SetUsbRndisPacketFilter;
UsbRndisDevice->UsbEth.GetUsbEthStatistic = GetRndisStatistic;
// ------- Below is for UNDI Hook
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiGetState =
RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiStart = RndisUndiStart;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiStop = RndisUndiStop;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiGetInitInfo =
RndisUndiGetInitInfo;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiGetConfigInfo =
RndisUndiGetConfigInfo;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiInitialize =
RndisUndiInitialize;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiReset = RndisUndiReset;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiShutdown =
RndisUndiShutdown;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiInterruptEnable =
RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiReceiveFilter =
RndisUndiReceiveFilter;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiStationAddress =
RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiStatistics = NULL;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiMcastIp2Mac =
RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiNvData =
RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiGetStatus =
RndisUndiGetStatus;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiFillHeader =
RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiTransmit = NULL;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiReceive = NULL;
UndiStart (
IN PXE_CDB *Cdb,
IN NIC_DATA *Nic
)
{
PXE_CPB_START_31 *Cpb;
EFI_STATUS Status;
if ((Cdb->OpCode != PXE_OPCODE_START) ||
(Cdb->StatCode != PXE_STATCODE_INITIALIZE) ||
(Cdb->StatFlags != PXE_STATFLAGS_INITIALIZE) ||
(Cdb->IFnum >= (gPxe->IFcnt | gPxe->IFcntExt << 8)) ||
(Cdb->CPBsize != sizeof (PXE_CPB_START_31)) ||
(Cdb->DBsize != PXE_DBSIZE_NOT_USED) ||
(Cdb->DBaddr != PXE_DBADDR_NOT_USED) ||
(Cdb->OpFlags != PXE_OPFLAGS_NOT_USED))
{
Cdb->StatFlags = PXE_STATFLAGS_COMMAND_FAILED;
Cdb->StatCode = PXE_STATCODE_INVALID_CDB;
return;
} else {
Cdb->StatFlags = PXE_STATFLAGS_COMMAND_COMPLETE;
Cdb->StatCode = PXE_STATCODE_SUCCESS;
}
if (Nic->State != PXE_STATFLAGS_GET_STATE_STOPPED) {
Cdb->StatFlags = PXE_STATFLAGS_COMMAND_FAILED;
Cdb->StatCode = PXE_STATCODE_ALREADY_STARTED;
return;
}
Cpb = (PXE_CPB_START_31 *)(UINTN)Cdb->CPBaddr;
Nic->PxeStart.Delay = Cpb->Delay;
Nic->PxeStart.Virt2Phys = Cpb->Virt2Phys;
Nic->PxeStart.Block = Cpb->Block;
Nic->PxeStart.Map_Mem = 0;
Nic->PxeStart.UnMap_Mem = 0;
Nic->PxeStart.Sync_Mem = Cpb->Sync_Mem;
Nic->PxeStart.Unique_ID = Cpb->Unique_ID;
Nic->State = PXE_STATFLAGS_GET_STATE_STARTED;
if (Nic->UsbEth->UsbEthUndi.UsbEthUndiStart != NULL) {
Status = Nic->UsbEth->UsbEthUndi.UsbEthUndiStart (Cdb, Nic); <---
RndisUndiStart
if (EFI_ERROR (Status)) {
Cdb->StatFlags = PXE_STATFLAGS_COMMAND_FAILED;
}
}
}
Thanks,
Richard
-----Original Message-----
From: [email protected] <[email protected]> On Behalf Of Chang, Abner via
groups.io
Sent: 2022年9月12日 3:56 PM
To: RichardHo [何明忠] <[email protected]>; [email protected];
[email protected]
Cc: Andrew Fish <[email protected]>; Leif Lindholm <[email protected]>;
Michael D Kinney <[email protected]>; Michael Kubacki
<[email protected]>; Zhiguang Liu <[email protected]>; Liming
Gao <[email protected]>; TonyLo [羅金松] <[email protected]>
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH] UsbNetworkPkg: add USB network
devices support
**CAUTION: The e-mail below is from an external source. Please exercise caution
before opening attachments, clicking links, or following guidance.**
[AMD Official Use Only - General]
Hi Richard, thanks for the reply. My feedback in line below,
> -----Original Message-----
> From: RichardHo [何明忠] <[email protected]>
> Sent: Monday, September 12, 2022 11:48 AM
> To: Chang, Abner <[email protected]>; [email protected];
> [email protected]
> Cc: Andrew Fish <[email protected]>; Leif Lindholm
> <[email protected]>; Michael D Kinney
> <[email protected]>; Michael Kubacki
> <[email protected]>; Zhiguang Liu
> <[email protected]>; Liming Gao <[email protected]>;
> TonyLo [羅金松]
> <[email protected]>
> Subject: RE: [EXTERNAL] RE: [edk2-devel] [PATCH] UsbNetworkPkg: add
> USB network devices support
>
> [CAUTION: External Email]
>
> Hi Abner,
>
> Ans #1. I think the USB network feature is better in the
> UsbNetworkPkg. It is easy to control USB network stack in this package.
> The package is same as NetworkPkg(The NetworkPkg gathers
> all network stack features). The UsbNetworkPkg could gather the USB
> network stack.
UsbNetworkPkg has the feature that overlaps with Bus\Usb and \NetworkPkg in my
opinion, that is the reason I think we can have these modules in the existing
package. Plus as I know from the edk2 community view point, we only create a
new package when necessary for a totally new feature. However, this is at the
discretion of edk2 stewards. You can confirm this with them.
> Ans #2. OK. I will rename it next.
> Ans #4 Other driver could use EFI_OPEN_PROTOCOL_BY_DRIVER with
> USB_ETHERNET_PROTOCOL to own on the specific USB CDC device.
> But, the specific USB CDC driver need to install the
> binding driver before NetworkCommon driver.
My question was USB_ETHERNET_PROTOCOL is designed as an abstract protocol, with
this the underlying transport (UsbIo or PciIo) is agnostic to the upper layer
driver. Upper layer driver (e.g., USB_ETHERNET_PROTOCOL based SNP driver) can
check whether USB_ETHERNET_PROTOCOL is installed on the given Controller handle
at Supported() function. However, seems the upper layer driver has no way to
distinguish the USB CDC driver interface class if I read your code right; then
manages the right device (in the case of multiple USB CDC devices are attached
to the system). We can either attach USB Class Device Path to the device path
in Usb CDC driver or provide a function to retrieve the USB CDC interface
class/subclass/protocol via USB_ETHERNET_PROTOCOL. I would prefer the former
one.
Thanks
Abner
> Ans #5 NetworkCommon driver build a Mac device path that link PciIo
> device path.
> So, SNP driver could found PciIo device path when
> NetworkCommon driver to install the EFI_NETWORK_INTERFACE_IDENTIFIER_PROTOCOL.
>
> Thanks,
> Richard
>
> -----Original Message-----
> From: Chang, Abner <[email protected]>
> Sent: 2022年9月9日 3:07 PM
> To: [email protected]; [email protected]; RichardHo [何明忠]
> <[email protected]>
> Cc: Andrew Fish <[email protected]>; Leif Lindholm
> <[email protected]>; Michael D Kinney
> <[email protected]>; Michael Kubacki
> <[email protected]>; Zhiguang Liu
> <[email protected]>; Liming Gao <[email protected]>;
> TonyLo [羅金松]
> <[email protected]>
> Subject: [EXTERNAL] RE: [edk2-devel] [PATCH] UsbNetworkPkg: add USB
> network devices support
>
>
> **CAUTION: The e-mail below is from an external source. Please
> exercise caution before opening attachments, clicking links, or
> following guidance.**
>
> [AMD Official Use Only - General]
>
> Hi Richard,
> That is pretty hard to give the in-line comment in this huge patch
> email. You would be also hard to find the comments given to each
> module or the protocol header file.
> So fist all of, please organize your change into several patches. For
> example, 1. UsbEthernetProtocol.h 2. UsbRndis module 3. UsbCdcNcm module 4.
> UsbCdcEcm module 5. NetworkCommon module
>
> Some rough feedbacks to this change before I giving the comments to
> each patch after you resending the new patch set.
> 1. I suggest having UsbNetwork folder under MdeModulePkg/Usb/. Then
> under UsbNetwork, you can have those four USB network related modules.
> However, we can Rename NetwrokCommon to UsbNetwork if your are ok with
> it.
> 2. UsbEhernetProtocol is not an EFI protocol, so please add EdkII as
> the prefix to UsbEthernetProtocol, it would be
> EdkIIUsbEthernetProtocol. And we can have UsbEthernetProtocol.h under
> MdeModulePkg/Include/Protocol 4.
> From this change, NetwrokCommon driver listens to
> USB_ETHERNET_PROTOCOL and install EFI NII protocol for each instance.
> However, the upper layer driver wouldn't know the CDC interface
> class/subclass/protocol if it only listen to USB_ETHERNET_PROTOCOL and
> install its own SNP on the specific USB CDC device; in the case if
> there are multiple USB CDC devices attached on the system. Is my
> understanding correct?
> 5. I don’t see the UsbEthernetProtocol based SNP protocol is installed
> and the SNP driver under NetworkPkg has the dependency with PciIo. Do
> we have to implement UsbEthernetProtocol based SNP by our own?
>
> I will check the functionality on our platform if I get the chance.
> Thanks
> Abner
>
>
> > -----Original Message-----
> > From: [email protected] <[email protected]> On Behalf Of
> Rebecca
> > Cran via groups.io
> > Sent: Saturday, September 3, 2022 5:48 AM
> > To: [email protected]; [email protected]
> > Cc: Andrew Fish <[email protected]>; Leif Lindholm
> > <[email protected]>; Michael D Kinney
> > <[email protected]>; Michael Kubacki
> > <[email protected]>; Zhiguang Liu
> > <[email protected]>; Liming Gao <[email protected]>;
> > TonyLo [羅金松]
> > <[email protected]>
> > Subject: Re: [edk2-devel] [PATCH] UsbNetworkPkg: add USB network
> > devices support
> >
> > [CAUTION: External Email]
> >
> > I've pushed this patch to a branch at
> >
> https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnam1
> 1.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fnam&
> amp;data=05%7C01%7Cigork%40ami.com%7C2dbbb37cd5aa464372d408da94944389%
> 7C27e97857e15f486cb58e86c2b3040f93%7C1%7C0%7C637985661785139437%7CUnkn
> own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=6kJ7djzYAbWay75nEMfH2eATKEWC2
> j%2FoFYakFE2PuJw%3D&reserved=0
> 1
> >
> 2.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fn
> am1
> >
> &data=05%7C01%7CAbner.Chang%40amd.com%7C92e0dc34ec2a4c9a9f
> 6908da94
> >
> 71a29e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63798551306
> 1024727
> >
> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL
> CJBTiI6I
> >
> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=78%2FyX9SBP
> 4wrftOYrmnP
> > muF5e%2F3NsmqY7JVw4atcoGM%3D&reserved=0
> >
> 1.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fgi
> th
> >
> &data=05%7C01%7Crichardho%40ami.com%7C1ffd2a8dbf6f4cda9da908
> da9231
> >
> e426%7C27e97857e15f486cb58e86c2b3040f93%7C1%7C0%7C63798304023249
> 3161%7
> >
> CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJ
> BTiI6Ik1
> >
> haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=XK3dw8vGMtln2
> Ig6Rcieyw6l
> > orLdsvy2Oko9gAllSK0%3D&reserved=0
> > ub.com%2Fbcran%2Fedk2%2Ftree%2Fusb-
> >
> net&data=05%7C01%7Cabner.chang%40amd.com%7C2c812dc15b9542b
> >
> 8a64308da8d2cdf2b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C
> >
> 637977521120662673%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
> >
> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7
> >
> C&sdata=ZFcSw3AL0MnljvyODrv1dYAL8LuadwCYe65xhE1hXWY%3D&a
> > mp;reserved=0 .
> >
> > --
> > Rebecca Cran
> >
> > On 9/1/22 23:24, RichardHo [何明忠] via groups.io wrote:
> > > UsbNetworkPkg provides network functions for USB ACM, USB NCM,
> and
> > USB
> > > RNDIS network device.
> > >
> > > Signed-off-by: Richard Ho <[email protected]>
> > > Cc: Andrew Fish <[email protected]>
> > > Cc: Leif Lindholm <[email protected]>
> > > Cc: Michael D Kinney <[email protected]>
> > > Cc: Michael Kubacki <[email protected]>
> > > Cc: Zhiguang Liu <[email protected]>
> > > Cc: Liming Gao <[email protected]>
> > > Reviewed-by: Tony Lo <[email protected]>
> > > ---
> > > UsbNetworkPkg/Config/UsbNetworkPkg.inc.dsc | 9 +
> > > .../Config/UsbNetworkPkgComponentsDxe.inc.dsc | 20 +
> > > .../Config/UsbNetworkPkgComponentsDxe.inc.fdf | 20 +
> > > .../Config/UsbNetworkPkgDefines.inc.dsc | 23 +
> > > .../Include/Protocol/UsbEthernetProtocol.h | 872 +++++++++
> > > UsbNetworkPkg/NetworkCommon/ComponentName.c | 264 +++
> > > UsbNetworkPkg/NetworkCommon/DriverBinding.c | 583 ++++++
> > > UsbNetworkPkg/NetworkCommon/DriverBinding.h | 263 +++
> > > UsbNetworkPkg/NetworkCommon/NetworkCommon.inf | 43 +
> > > UsbNetworkPkg/NetworkCommon/PxeFunction.c | 1734
> > +++++++++++++++++
> > > UsbNetworkPkg/ReadMe.md | 65 +
> > > UsbNetworkPkg/ReleaseNotes.md | 11 +
> > > UsbNetworkPkg/UsbCdcEcm/ComponentName.c | 170 ++
> > > UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.c | 504 +++++
> > > UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.h | 211 ++
> > > UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.inf | 41 +
> > > UsbNetworkPkg/UsbCdcEcm/UsbEcmFunction.c | 861 ++++++++
> > > UsbNetworkPkg/UsbCdcNcm/ComponentName.c | 170 ++
> > > UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.c | 508 +++++
> > > UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.h | 245 +++
> > > UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.inf | 41 +
> > > UsbNetworkPkg/UsbCdcNcm/UsbNcmFunction.c | 946 +++++++++
> > > UsbNetworkPkg/UsbNetworkPkg.dec | 32 +
> > > UsbNetworkPkg/UsbRndis/ComponentName.c | 172 ++
> > > UsbNetworkPkg/UsbRndis/UsbRndis.c | 848 ++++++++
> > > UsbNetworkPkg/UsbRndis/UsbRndis.h | 569 ++++++
> > > UsbNetworkPkg/UsbRndis/UsbRndis.inf | 41 +
> > > UsbNetworkPkg/UsbRndis/UsbRndisFunction.c | 1587
> > +++++++++++++++
> > > 28 files changed, 10853 insertions(+)
> > > create mode 100644 UsbNetworkPkg/Config/UsbNetworkPkg.inc.dsc
> > > create mode 100644
> > UsbNetworkPkg/Config/UsbNetworkPkgComponentsDxe.inc.dsc
> > > create mode 100644
> > UsbNetworkPkg/Config/UsbNetworkPkgComponentsDxe.inc.fdf
> > > create mode 100644
> > UsbNetworkPkg/Config/UsbNetworkPkgDefines.inc.dsc
> > > create mode 100644
> > UsbNetworkPkg/Include/Protocol/UsbEthernetProtocol.h
> > > create mode 100644
> > UsbNetworkPkg/NetworkCommon/ComponentName.c
> > > create mode 100644 UsbNetworkPkg/NetworkCommon/DriverBinding.c
> > > create mode 100644 UsbNetworkPkg/NetworkCommon/DriverBinding.h
> > > create mode 100644
> > UsbNetworkPkg/NetworkCommon/NetworkCommon.inf
> > > create mode 100644 UsbNetworkPkg/NetworkCommon/PxeFunction.c
> > > create mode 100644 UsbNetworkPkg/ReadMe.md
> > > create mode 100644 UsbNetworkPkg/ReleaseNotes.md
> > > create mode 100644 UsbNetworkPkg/UsbCdcEcm/ComponentName.c
> > > create mode 100644 UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.c
> > > create mode 100644 UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.h
> > > create mode 100644 UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.inf
> > > create mode 100644 UsbNetworkPkg/UsbCdcEcm/UsbEcmFunction.c
> > > create mode 100644 UsbNetworkPkg/UsbCdcNcm/ComponentName.c
> > > create mode 100644 UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.c
> > > create mode 100644 UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.h
> > > create mode 100644 UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.inf
> > > create mode 100644 UsbNetworkPkg/UsbCdcNcm/UsbNcmFunction.c
> > > create mode 100644 UsbNetworkPkg/UsbNetworkPkg.dec
> > > create mode 100644 UsbNetworkPkg/UsbRndis/ComponentName.c
> > > create mode 100644 UsbNetworkPkg/UsbRndis/UsbRndis.c
> > > create mode 100644 UsbNetworkPkg/UsbRndis/UsbRndis.h
> > > create mode 100644 UsbNetworkPkg/UsbRndis/UsbRndis.inf
> > > create mode 100644 UsbNetworkPkg/UsbRndis/UsbRndisFunction.c
> >
> >
> >
> >
> >
> -The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI). This communication is
> intended to be read only by the individual or entity to whom it is
> addressed or by their designee. If the reader of this message is not
> the intended recipient, you are on notice that any distribution of
> this message, in any form, is strictly prohibited. Please promptly
> notify the sender by reply e-mail or by telephone at 770-246-8600, and
> then delete or destroy all copies of the transmission.
-The information contained in this message may be confidential and proprietary
to American Megatrends (AMI). This communication is intended to be read only by
the individual or entity to whom it is addressed or by their designee. If the
reader of this message is not the intended recipient, you are on notice that
any distribution of this message, in any form, is strictly prohibited. Please
promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and
then delete or destroy all copies of the transmission.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94028): https://edk2.groups.io/g/devel/message/94028
Mute This Topic: https://groups.io/mt/93121092/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
