From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-x230.google.com (mail-lf0-x230.google.com [IPv6:2a00:1450:4010:c07::230]) (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 4179281E91 for ; Thu, 19 Jan 2017 05:49:07 -0800 (PST) Received: by mail-lf0-x230.google.com with SMTP id k86so36785336lfi.0 for ; Thu, 19 Jan 2017 05:49:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=eH7bstBYpQtSPkeO7AwUsMyEU3kHFN0DWw6nlMH8BtA=; b=JlDHbXR3CHZbtSqedQDZJNiGx3vRTnAz8jE3KhUu7PkhG0Lx12u8SRi8DxvBhCgr4E L81QUDdFy0yb+KFJ5BHVxeQEP3v7D5SOPWzKuyNZH/PWRGtwnTzIVdGbb1Aq66dRE/PI +B32+ozEJ9sXHvcEVKBMfMFcC1mmwSa9Eu+H8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=eH7bstBYpQtSPkeO7AwUsMyEU3kHFN0DWw6nlMH8BtA=; b=moX1OJQwtfTIXcwdmsMLbeKSbZaot4wosBei5zMermyAAQyo77lBa06jQgQ+qfIkAn O5/J+/BKUkEvDWo7Qb2oQ/i1zcyfYfYPPBO++7YMkfKEZnciXP4QxSzvC3BMcr+8cWzq 2J+TGXTFuHPqf4xNBv8392AuVuSlk5EgjHYOJ3qj7k9a54aXm4sB2kmBjD7gLb4i5kFc +Z2wJb3dYCbIIeQjKp7HXDuyYQMFB/8gm0pgg76CGjxz1QqvLv+xdM6UUH7K2W3rJKuD niyT/Fia7QmgxgPYptvOMxvaY6Rcuur8tRuZQfUv+B3ccsHO3OezUSBl89StU3gPngac ajbA== X-Gm-Message-State: AIkVDXIiSjapeiiUCiMUw9EW2va+wdPTwRcom4MuZSOaE8tm0L3fGulSUw5FRxReBoth4h7sa42WMcaVOw9OzYFb X-Received: by 10.46.21.72 with SMTP id 8mr4250871ljv.11.1484833745165; Thu, 19 Jan 2017 05:49:05 -0800 (PST) MIME-Version: 1.0 Received: by 10.25.207.72 with HTTP; Thu, 19 Jan 2017 05:49:04 -0800 (PST) In-Reply-To: <1484782032-82342-1-git-send-email-daniil.egranov@arm.com> References: <1484782032-82342-1-git-send-email-daniil.egranov@arm.com> From: Ryan Harkin Date: Thu, 19 Jan 2017 13:49:04 +0000 Message-ID: To: Daniil Egranov Cc: "edk2-devel@lists.01.org" , Leif Lindholm Subject: Re: [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Fixed crash on Juno R0 X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Jan 2017 13:49:07 -0000 Content-Type: text/plain; charset=UTF-8 On 18 January 2017 at 23:27, Daniil Egranov wrote: > The Marvell Yukon MAC address load supported only on Juno R1 and R2. > It disabled for Juno R0 due to PCI issues on this board. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Daniil Egranov Tested-by: Ryan Harkin > --- > ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c > index 47ff587..e9e6990 100644 > --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c > +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c > @@ -378,6 +378,7 @@ OnEndOfDxe ( > EFI_DEVICE_PATH_PROTOCOL* PciRootComplexDevicePath; > EFI_HANDLE Handle; > EFI_STATUS Status; > + UINT32 JunoRevision; > > // > // PCI Root Complex initialization > @@ -393,8 +394,12 @@ OnEndOfDxe ( > Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath, FALSE); > ASSERT_EFI_ERROR (Status); > > - Status = ArmJunoSetNicMacAddress (); > - ASSERT_EFI_ERROR (Status); > + GetJunoRevision (JunoRevision); > + > + if (JunoRevision != JUNO_REVISION_R0) { > + Status = ArmJunoSetNicMacAddress (); > + ASSERT_EFI_ERROR (Status); This is just an FYI, but I stacked your patch on top of mainline, like this: 5f81f61 2017-01-18 ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Fixed crash on Juno R0 [Daniil Egranov] 19ca06b 2017-01-19 OvmfPkg: Remove superfluous return statements. [Thomas Huth] The first time I ran this, Juno R0 worked fine, but on R1 and R2, the assert triggered: UEFI firmware (version 5f81f61 built at 11:56:52 on Jan 19 2017) [snip] ASSERT_EFI_ERROR (Status = Not Found) ASSERT [ArmJunoDxe] /linaro/platforms/uefi/edk2/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c(401): !EFI_ERROR (Status) I worked out what is happening. And it's not to do with this patch. It's another fall-out from the re-work you did to the previous patch. It's also ultimately due to a bug the firmware. With the initial version of your "Set Marvell Yukon MAC address" patch, this hang didn't happen. I suspect that was because your error checking was weaker and certain PCIe failures didn't trigger the assert. To reproduce the error with this commit: 1) power on and boot R1 or R2 into Shell I do this by interrupting the boot by pressing ESCAPE and using the boot menu 2) At the Shell prompt, run "reset -s" to shutdown 3) At the ARM Boot Loader "Cmd>" prompt, run "reboot" 4) the board will hang while booting UEFI, assuming the board firmware doesn't die with constant messages like this: ERROR: PCIe CSR read failed to respond ERROR: SMBus transaction not claimed Assuming the problem is firmware, not EDK2, what should we do about it? Prior to your "Set Marvell Yukon MAC address" patch, or with the earlier version, the board would boot anyway, but the Yukon device would be missing. Now it dies. I don't know which is worse, but I think hanging is worse than an ethernet port dropping out. Although hanging is a bit more obvious that there's a problem... > + } > } > > STATIC > -- > 2.7.4 >