From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by mx.groups.io with SMTP id smtpd.web12.9719.1578582133342797608 for ; Thu, 09 Jan 2020 07:02:13 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=thobz13N; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.128.66, mailfrom: pete@akeo.ie) Received: by mail-wm1-f66.google.com with SMTP id p17so3214272wmb.0 for ; Thu, 09 Jan 2020 07:02:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=LbZXSXfMPVlmKJpj7HUjeYKLe0n5taJjjPFbUt9nFAA=; b=thobz13N2Bzn0r3OGWLYd8AAIokh+/9Mz4bNRAO/C4pRzfomnhMmdNPSwZEq7TdK8G dUx4koLCjiOfvWmf1+qOOOgLv1xnW0H+w4sRtwTpdk6fzAj+BKxY+GQX2lin/3BmdJDx UV0uzM0Z3y3If9GmrV284IORLP8cH6WER5foLlUjCF7xpubhod1HiTBPOQt1Sd6gz7nV PHh5BP6SZosL0ZW6uxmWdogsz+SkSV22jDFqxS48J1GglK7djhcVqCQ6mbhsCiUsE1/j FTs/GfLLktPlwKDm1xWjcxUMQOxnUU0C8yXwlUUr2q0buecHRhTIARZamuSU/uSgcCin 5yTA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=LbZXSXfMPVlmKJpj7HUjeYKLe0n5taJjjPFbUt9nFAA=; b=VzAIOTmFrwRq7ESvbqq4YsqKx79e3z0e01EtbVzyQ2H4A11Oc6dZrF+q/L2f0ueXML gqAsgdTRecgCP91cRIhB3m+0EOwHQBL8shE9zKy6/oKdvLj0HTVV1L3NG5ZCKPhYMqkr bOVChk37rvmHSJtHF2k/8gXXXmGUmEkXA90l9PinBUCi0avOgi+xWVoz1ZJAKwN28/0b Uf4KKfLSYvoin7TVEIIo880t/PNrM7z1Ddkk8Y6BjxibeXCFlt0w8ItwgUhkfCAq/OiU nqrKEORiDqDM6Hchr76SjJDt9rR6Oo4SIEDcGVPXb5hcGE2VUG/74s/YeifiULyv9qeq ng5w== X-Gm-Message-State: APjAAAWg++4CvpjOjaUtVXWqWnsFSsXgkxlyDqKV93usmHQ4F4yMzJ4I vNh5rpmAppuHWt/GpbHpxQodUQ== X-Google-Smtp-Source: APXvYqzWXVI8QHsPkLVLMwObEoqL8PBTFT13T50h0MtUO+04OBZRi9j6EwpgTDmAIqRyroLCAE81Ww== X-Received: by 2002:a7b:c851:: with SMTP id c17mr5681881wml.71.1578582131670; Thu, 09 Jan 2020 07:02:11 -0800 (PST) Return-Path: Received: from [10.0.0.122] ([84.203.81.176]) by smtp.googlemail.com with ESMTPSA id z124sm3257060wmc.20.2020.01.09.07.02.08 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 09 Jan 2020 07:02:09 -0800 (PST) Subject: Re: [edk2-platforms][PATCH 2/2] Platform/RPi/RPiFirmwareDxe: Fix serial number population for RPi4 To: Ard Biesheuvel Cc: edk2-devel-groups-io , Leif Lindholm , =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= References: <20200108170004.6680-1-pete@akeo.ie> <20200108170004.6680-3-pete@akeo.ie> From: "Pete Batard" Message-ID: <787fa8df-69a2-9aba-8296-8a6e14a8af4a@akeo.ie> Date: Thu, 9 Jan 2020 15:02:07 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Hi Ard, On 2020.01.09 14:44, Ard Biesheuvel wrote: > On Wed, 8 Jan 2020 at 18:00, Pete Batard wrote: >> >> Some (all?) Raspbery Pi 4 platforms report 0x0000000010000000 as their >> board serial when queried through the VideoCore mailbox. >> >> Fix this by using the MAC address then. >> >> Signed-off-by: Pete Batard >> --- >> Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c >> index dd61ef089ca7..75826fdc0e53 100644 >> --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c >> +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c >> @@ -394,8 +394,9 @@ RpiFirmwareGetSerial ( >> } >> >> *Serial = Cmd->TagBody.Serial; >> - // Some platforms return 0 for serial. For those, try to use the MAC address. >> - if (*Serial == 0) { >> + // Some platforms return 0 or 0x0000000010000000 for serial. >> + // For those, try to use the MAC address. >> + if ((*Serial == 0) || ((*Serial & 0xFFFFFFFF0FFFFFFFULL) == 0)) { > > What is the point of using a mask here? Is it deliberately matching > 0x0000000020000000 or 0x00000000F0000000 as well? My expectation is that if we're seeing 0x0000000010000000 as a board serial, there's not so insignificant chance are future boards may use 0x0000000010000000, 0x0000000030000000 and so on. In other words, when someone starts counting from 0 to 1 somewhere, it's not unreasonable to also expect them to reach 2 at some stage... Now, if anything, the one change we could apply here is remove the (*Serial == 0) check, which of course is redundant. But I fail to see why we'd want to restrict ourselves to checking only for 0x0000000010000000 here, when we have no clear idea how the value we read may evolve in the future, as anything that the mask condition matches is clearly not a serial we want to use anyway... Regards, /Pete > >> Status = RpiFirmwareGetMacAddress ((UINT8*) Serial); >> // Convert to a more user-friendly value >> *Serial = SwapBytes64 (*Serial << 16); >> -- >> 2.21.0.windows.1 >>