From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::243; helo=mail-it0-x243.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-it0-x243.google.com (mail-it0-x243.google.com [IPv6:2607:f8b0:4001:c0b::243]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8E4BA2194D387 for ; Thu, 6 Sep 2018 07:27:01 -0700 (PDT) Received: by mail-it0-x243.google.com with SMTP id u13-v6so14392268iti.1 for ; Thu, 06 Sep 2018 07:27:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=CWt2HWdtiK+U58UM15Rbb1ciidkWe5+F4EZJLHwCvxQ=; b=fzkWwqPtBK9yjpgUn7+E9pU5G3VxCNV/bw90HJ8qGZJh6ymlwLD1iIBSvJI/cZWI3b Sw2KQOAHjsg26UYzx2EpdBCWIQljMciuab3rAElxrwQkowDrSKFdbVi0K1zBWj5oDZ/a MKmMiYELuW+WEfq8QKIb0IGRf3mCC+YIjcXScLCq1kLOxAApjRvVNvw8bKYmHb/GHZ5+ Qd2cHbDISqZyoVUWONBOvKDg6SFUgQ1Wq+CpOrzJoSdb1Lj6wuIOf+imbtKZoFzuHiD+ EYmx9SqYu1UHJI3mnfR6kgqTLjMGCMIW4DKyZ3OndKT1REILjEEwEGsiZ/o/9yy6Mn0b aZUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=CWt2HWdtiK+U58UM15Rbb1ciidkWe5+F4EZJLHwCvxQ=; b=DbfO9ijm+rSkth3ZHzTQfUnyDlWiu0zveOd+rwKgExrbukEec6qQ6GccR0Eq7V3Let /oq4Sn5xSrRpBl0JwOjnCt6M/bDt6j384sO0cTuz7D/Xx1SQYb7n2uXIbMZQ2x4slXNc h14Nq+Z7Eh1py86lAg1g7D1ijm4MiwgnqyR2bhaBK7xGbWkSjye854AL6GRUn/sBk5ZN J4CXORSevaIv2QvMjcmeJIpmLoUMXzhU+lc52Els5C7HFTHIKC6bBBthHN8WwifTP+cU 1jwN0dysw5hWk8fbTmODD2K/EZfkg7619KenEb6lWl7OtcfOBL+gco1js1mheEXLSs4u /vlw== X-Gm-Message-State: APzg51ANvqnPiLGtOOiKPuJJnJ8TBM1P3DCPtngpLOPoQeObcGKskDbO utmrkEvHfi2uCqC5qCvieX9fUpyr8tkbZF0m5pjrjQ== X-Google-Smtp-Source: ANB0VdbCS/a74Q8dX1vrsnWrR3ApjrpIZImAK05gD892oGCirpr3XHh3jMd8MZhReMdEHBBKM94U6kukGSpLkS3msIw= X-Received: by 2002:a24:e084:: with SMTP id c126-v6mr2750478ith.136.1536244020848; Thu, 06 Sep 2018 07:27:00 -0700 (PDT) MIME-Version: 1.0 References: <1535950443-27106-1-git-send-email-mw@semihalf.com> <1535950443-27106-2-git-send-email-mw@semihalf.com> In-Reply-To: From: Marcin Wojtas Date: Thu, 6 Sep 2018 16:26:48 +0200 Message-ID: To: Ard Biesheuvel Cc: edk2-devel-01 , Leif Lindholm , nadavh@marvell.com, "jsd@semihalf.com" , Grzegorz Jaszczyk , Tomasz Michalec Subject: Re: [platforms: PATCH 1/7] Silicon/SynQuacer/PlatformDxe: Modify initialization of SdMmcOverride X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Sep 2018 14:27:01 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable czw., 6 wrz 2018 o 16:04 Ard Biesheuvel napisa= =C5=82(a): > > On 3 September 2018 at 06:53, Marcin Wojtas wrote: > > From: Tomasz Michalec > > > > This patch changes way the EDKII_SD_MMC_OVERRIDE protocol > > sturcture is allocated. Using AllocateZeroPool and then > > seting callbacks in the structure allow driver to be immune to > > adding new callbacks in SdMmcOveride protocol in future. > > > > What is the point of this patch? > > Statically allocating the structure will zero initialize the members > that are not initialized explicitly, but only the members that are > known to exist at compile time. > In such case this patch is really not needed. > I guess the idea of this patch is to work around the latter > limitation, but unfortunately, using sizeof(EDKII_SD_MMC_OVERRIDE) > puts you in the exact same situation. If the newly added callback are zero-initialized, the situation is fine as they won't be executed. > > This is the reason I added the version field. New hooks should only be > added after incrementing the version, and calling the new hooks should > only occur if the runtime version of the protocol implementation is > greater than or equal to the version where those hooks were first > introduced. > So even if the given SdMmcOverride protocol callback will be NULL for Synquacer controller, is there still a risk that anything could be broken without the version check? Best regards, Marcin > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Marcin Wojtas > > --- > > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c | 20 ++++++++++= +++------- > > 1 file changed, 13 insertions(+), 7 deletions(-) > > > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c b/S= ilicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c > > index e0987c9..1ad8b88 100644 > > --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c > > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c > > @@ -59,6 +59,8 @@ > > > > STATIC EFI_HANDLE mSdMmcControllerHandle; > > > > +STATIC EDKII_SD_MMC_OVERRIDE *mSdMmcOverride; > > + > > STATIC EFI_ACPI_DESCRIPTION_HEADER *mSsdt; > > STATIC UINTN mSsdtSize; > > STATIC VOID *mEventRegistration; > > @@ -180,12 +182,6 @@ SynQuacerSdMmcNotifyPhase ( > > return EFI_SUCCESS; > > } > > > > -STATIC EDKII_SD_MMC_OVERRIDE mSdMmcOverride =3D { > > - EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION, > > - SynQuacerSdMmcCapability, > > - SynQuacerSdMmcNotifyPhase, > > -}; > > - > > STATIC > > VOID > > EFIAPI > > @@ -255,10 +251,20 @@ RegisterEmmc ( > > SYNQUACER_EMMC_BASE, SYNQUACER_EMMC_BASE_SZ); > > ASSERT_EFI_ERROR (Status); > > > > + mSdMmcOverride =3D AllocateZeroPool (sizeof (EDKII_SD_MMC_OVERRIDE))= ; > > + if (mSdMmcOverride =3D=3D NULL) { > > + DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__)= ); > > + return EFI_OUT_OF_RESOURCES; > > + } > > + > > + mSdMmcOverride->Version =3D EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION; > > + mSdMmcOverride->Capability =3D SynQuacerSdMmcCapability; > > + mSdMmcOverride->NotifyPhase =3D SynQuacerSdMmcNotifyPhase; > > + > > Handle =3D NULL; > > Status =3D gBS->InstallProtocolInterface (&Handle, > > &gEdkiiSdMmcOverrideProtocolGuid, > > - EFI_NATIVE_INTERFACE, (VOID **)&mSdMmcOverride); > > + EFI_NATIVE_INTERFACE, mSdMmcOverride); > > ASSERT_EFI_ERROR (Status); > > > > return EFI_SUCCESS; > > -- > > 2.7.4 > >