Message ID | B1FF2E9001CE9041BD10B825821D5BC504A0A868@SHSMSX101.ccr.corp.intel.com |
---|---|
State | New |
Headers | show |
On 9 October 2013 02:53, Fu, Siyuan <siyuan.fu@intel.com> wrote: > Hi, Pollack > > I don't agree with the opinion that the Snp.Mode can't be accessed before > Snp->Initialize() is called. According to UEFI spec, all the fields in > Snp.Mode structure must be discovered during driver initialization, per my > understanding the words "driver initialization" mean the driver binding > start function, not Snp->Initialize() interface. And in EDKII > implementation, all these parameters in Snp.Mode are prepared by calling > PXE_OPCODE_GET_INIT_INFO in SimpleNetworkDriverStart() function before SNP > protocol is installed into the protocol database, so they are valid to use > once SNP protocol is produced. > > As with the other patch, yes, this was already discussed when Olivier originally posted this patch. I personally missed his re-posting of the patch with a new version, but I see that it has not had a reply on the list anyway. I think we should "withdraw" this patch submission and continue discussion on Olivier's post. > Fu, Siyuan > Best regards > > > -----Original Message----- > From: Reece R. Pollack [mailto:reece.pollack@linaro.org] > Sent: Tuesday, October 08, 2013 3:40 AM > To: edk2-devel@lists.sourceforge.net > Cc: patches@linaro.org > Subject: [edk2] [PATCH 02/10] MdeModulePkg//ArpDxe: Retrieved SnpMode only > after configuring Snp > > From: Olivier Martin <olivier.martin@arm.com> > > When Arp driver starts (with ArpDriverBindingStart()), its service will be > created and the Mnp child configured (ArpService->Mnp->Configure() called > in ArpCreateService()). > > It is only at this time the Snp protocol will be initialized (at the end > of MnpStart()). > So, a valid SnpMode could not be expected prior to > ArpService->Mnp->Configure(). > > Signed-off-by: Olivier Martin <olivier.martin@arm.com> > --- > MdeModulePkg/Universal/Network/ArpDxe/ArpDriver.c | 33 > +++++++++++---------- > 1 file changed, 17 insertions(+), 16 deletions(-) mode change 100644 => > 100755 MdeModulePkg/Universal/Network/ArpDxe/ArpDriver.c > > diff --git a/MdeModulePkg/Universal/Network/ArpDxe/ArpDriver.c > b/MdeModulePkg/Universal/Network/ArpDxe/ArpDriver.c > old mode 100644 > new mode 100755 > index 81ddd62..5cf717f > --- a/MdeModulePkg/Universal/Network/ArpDxe/ArpDriver.c > +++ b/MdeModulePkg/Universal/Network/ArpDxe/ArpDriver.c > @@ -103,22 +103,6 @@ ArpCreateService ( > } > > // > - // Get the underlayer Snp mode data. > - // > - Status = ArpService->Mnp->GetModeData (ArpService->Mnp, NULL, > &ArpService->SnpMode); > - if ((Status != EFI_NOT_STARTED) && EFI_ERROR (Status)) { > - goto ERROR_EXIT; > - } > - > - if (ArpService->SnpMode.IfType != NET_IFTYPE_ETHERNET) { > - // > - // Only support the ethernet. > - // > - Status = EFI_UNSUPPORTED; > - goto ERROR_EXIT; > - } > - > - // > // Set the Mnp config parameters. > // > ArpService->MnpConfigData.ReceivedQueueTimeoutValue = 0; @@ -141,6 > +125,23 @@ ArpCreateService ( > } > > // > + // Get the underlayer Snp mode data. Must do this after MNP > + configuration else some parameters // (e.g. current address) may not > + be set // Status = ArpService->Mnp->GetModeData (ArpService->Mnp, > + NULL, &ArpService->SnpMode); if ((Status != EFI_NOT_STARTED) && > + EFI_ERROR (Status)) { > + goto ERROR_EXIT; > + } > + > + if (ArpService->SnpMode.IfType != NET_IFTYPE_ETHERNET) { > + // > + // Only support the ethernet. > + // > + Status = EFI_UNSUPPORTED; > + goto ERROR_EXIT; > + } > + > + // > // Create the event used in the RxToken. > // > Status = gBS->CreateEvent ( > -- > 1.7.10.4 > > > > ------------------------------------------------------------------------------ > October Webinars: Code for Performance > Free Intel webinars can help you accelerate application performance. > Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most > from > the latest Intel processors and coprocessors. See abstracts and register > > http://pubads.g.doubleclick.net/gampad/clk?id=60134071&iu=/4140/ostg.clktrk > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > > ------------------------------------------------------------------------------ > October Webinars: Code for Performance > Free Intel webinars can help you accelerate application performance. > Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most > from > the latest Intel processors and coprocessors. See abstracts and register > > http://pubads.g.doubleclick.net/gampad/clk?id=60134071&iu=/4140/ostg.clktrk > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel >
diff --git a/MdeModulePkg/Universal/Network/ArpDxe/ArpDriver.c b/MdeModulePkg/Universal/Network/ArpDxe/ArpDriver.c old mode 100644 new mode 100755 index 81ddd62..5cf717f --- a/MdeModulePkg/Universal/Network/ArpDxe/ArpDriver.c +++ b/MdeModulePkg/Universal/Network/ArpDxe/ArpDriver.c @@ -103,22 +103,6 @@ ArpCreateService ( } // - // Get the underlayer Snp mode data. - // - Status = ArpService->Mnp->GetModeData (ArpService->Mnp, NULL, &ArpService->SnpMode); - if ((Status != EFI_NOT_STARTED) && EFI_ERROR (Status)) { - goto ERROR_EXIT; - } - - if (ArpService->SnpMode.IfType != NET_IFTYPE_ETHERNET) { - // - // Only support the ethernet. - // - Status = EFI_UNSUPPORTED; - goto ERROR_EXIT; - } - - // // Set the Mnp config parameters. // ArpService->MnpConfigData.ReceivedQueueTimeoutValue = 0; @@ -141,6 +125,23 @@ ArpCreateService (