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.2970.1576067987497018900 for ; Wed, 11 Dec 2019 04:39:48 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=fw1c8tNf; 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 p9so6913973wmc.2 for ; Wed, 11 Dec 2019 04:39:47 -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=3gO4MFw1j1TSKJQK2VQWVJJlq6OL8xM7zXAfPNMTe6I=; b=fw1c8tNf/odS2p9MUsWkevM9NO6H78jcelP+S+0iZNco/4wS/msy3KmGtrIb/Bcrbv Z5gAsydbQY0AFZMAZX4NmdxzI2DP5ATK+qDuqVv71K6DTjcAoXWIyhcYY4s08RFy2BL6 mAEEfTU0QKJhnxm8fSuth7R3rJ4Py4w+dBOqb+sirwwFdfTt+ShLF5miKGGW5ENNSPsY NtXVzg7bKHcA1XcE7Sf6dTwx4/XmFAxsTnnwsoPKEFtz/dkYkbNTEMz+JrZzd7sk+UZ2 0+tgm4GJfS6NNVCrpMRs1esphvLUWxqkI7F4T5ZWJWLXGk/2TDSsddSzkdlJSvu+miq8 qong== 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=3gO4MFw1j1TSKJQK2VQWVJJlq6OL8xM7zXAfPNMTe6I=; b=CX7mmBSwSWlXEdWyT4Ugzr0lQ6cvRTc/CkutoqFUGodAMUpxAFhhDbigIvmdD5aAZz MFARFXKKWYoMcpfWVW9j3z8uOU++IwSH5Yt4CU4Tj7GkNC8zWiTEqbq0osjPgZtcJzbH klAgvEYOzu5blnud21c5+AXvZr00Qjx0HK6BIDQBhuVDhq6hKBGnBUmbvtYBt6yfklXw jZs/p8RFUiLAuRqxum5o5Ge/GkbB6auWV7PVzMXPK+UcgNk5kfjECRZO/v5Bv7ZqqtLx LFdCIv9uDMAi2FsBj/MuS12nz9LRm+/4w+WQCiQ5R05g8xPzOV+xuwhJkKS5FXHl91UM 1OmA== X-Gm-Message-State: APjAAAXasP/i+1YumcYwUKazH2ycBO8/1R8PRyY1w/1X6fyFqn9NUs6A Qey0Pa5tKbzRlzOUOCHTj8WURQ== X-Google-Smtp-Source: APXvYqwicsRBJtzCRG8n9lby2x6KocvES73960LamKI3GZwxQcuWyoPnDhxmrcajh66e/BM0fWQO4A== X-Received: by 2002:a1c:3141:: with SMTP id x62mr3362777wmx.18.1576067985909; Wed, 11 Dec 2019 04:39:45 -0800 (PST) Return-Path: Received: from [10.0.0.122] ([84.203.45.230]) by smtp.googlemail.com with ESMTPSA id e16sm2066868wrs.73.2019.12.11.04.39.44 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 11 Dec 2019 04:39:45 -0800 (PST) Subject: Re: [edk2-platforms][PATCH 1/5] Platform/RPi: Fix overlap of SoC registers and RAM To: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , devel@edk2.groups.io Cc: ard.biesheuvel@linaro.org, leif.lindholm@linaro.org, andrey.warkentin@gmail.com, samer.el-haj-mahmoud@arm.com References: <20191211112552.15900-1-pete@akeo.ie> <20191211112552.15900-2-pete@akeo.ie> From: "Pete Batard" Message-ID: Date: Wed, 11 Dec 2019 12:39:43 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 8bit Hi Philippe, thanks for reviewing these. On 2019.12.11 12:21, Philippe Mathieu-Daudé wrote: > On 12/11/19 12:25 PM, Pete Batard wrote: >> From: Ard Biesheuvel >> >> Having RAM and SoC register regions overlap is problematic for MMIO, >> since, at the very least, we don't want these regions to be declared >> as cacheable. >> >> Signed-off-by: Pete Batard >> --- >>   Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c | 36 >> +++++++++++++------- >>   1 file changed, 23 insertions(+), 13 deletions(-) >> >> diff --git a/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c >> b/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c >> index cc761bea1307..781cf78b83d3 100644 >> --- a/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c >> +++ b/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c >> @@ -60,7 +60,7 @@ ArmPlatformGetVirtualMemoryMap ( >>   { >>     UINTN                         Index = 0; >>     UINTN                         GpuIndex; >> -  INT64                         ExtendedMemorySize; >> +  INT64                         SystemMemorySize; >>     ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable; >>     // Early output of the info we got from VideoCore can prove valuable. >> @@ -120,21 +120,21 @@ ArmPlatformGetVirtualMemoryMap ( >>     VirtualMemoryInfo[Index].Type             = RPI_MEM_RESERVED_REGION; >>     VirtualMemoryInfo[Index++].Name           = L"GPU Reserved"; >> -  // Compute the amount of extended RAM available on this platform >> -  ExtendedMemorySize = SIZE_256MB; >> -  ExtendedMemorySize <<= (mBoardRevision >> 20) & 0x07; >> -  ExtendedMemorySize -= SIZE_1GB; >> -  if (ExtendedMemorySize > 0) { >> -    VirtualMemoryTable[Index].PhysicalBase  = FixedPcdGet64 >> (PcdExtendedMemoryBase); >> -    VirtualMemoryTable[Index].VirtualBase   = >> VirtualMemoryTable[Index].PhysicalBase; >> -    VirtualMemoryTable[Index].Length        = ExtendedMemorySize; >> -    VirtualMemoryTable[Index].Attributes    = >> ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK; >> -    VirtualMemoryInfo[Index].Type           = RPI_MEM_BASIC_REGION; >> -    VirtualMemoryInfo[Index++].Name         = L"Extended System RAM"; >> -  } >> +  // Compute the total RAM size available on this platform >> +  SystemMemorySize = SIZE_256MB; >> +  SystemMemorySize <<= (mBoardRevision >> 20) & 0x07; > > TIL I learn this field is 3 bits (I thought it was 4). "32 GB should be enough for everybody" (which is the max RAM you'll be able to declare in 3 bits with the current scheme). Unless RAM becomes very very cheap, I doubt the Raspberry Pi foundation considers the current limit as much of a problem... > >> + >> +  // >> +  // Ensure that what we declare as System Memory doesn't overlap >> with the >> +  // Bcm2836 SoC registers. This can be achieved through a MIN () >> with the >> +  // base address since SystemMemoryBase is 0 (we assert if it isn't). > > Is the later comment "// On the Pi 3 the SoC registers may overlap > VideoCore => fix this > " still accurate? It is. We read the size of the GPU region (mVideoCoreSize) and the size of the System RAM from different queries. The thing is actually that VideoCore on the Pi 3 considers that its region should encompass the SoC registers, so it reports a size that does so, whereas we want to report them as separate regions (like they are reported on the Pi 4). Regards, /Pete >> +  // >> +  SystemMemorySize = MIN(SystemMemorySize, BCM2836_SOC_REGISTERS); >>     // Extended SoC registers (PCIe, genet, ...) >>     if (BCM2711_SOC_REGISTERS > 0) { >> +    // Same overlap protection as above for the Bcm2711 SoC registers >> +    SystemMemorySize                        = MIN(SystemMemorySize, >> BCM2711_SOC_REGISTERS); >>       VirtualMemoryTable[Index].PhysicalBase  = BCM2711_SOC_REGISTERS; >>       VirtualMemoryTable[Index].VirtualBase   = >> VirtualMemoryTable[Index].PhysicalBase; >>       VirtualMemoryTable[Index].Length        = >> BCM2711_SOC_REGISTER_LENGTH; >> @@ -155,6 +155,16 @@ ArmPlatformGetVirtualMemoryMap ( >>     VirtualMemoryInfo[Index].Type             = RPI_MEM_RESERVED_REGION; >>     VirtualMemoryInfo[Index++].Name           = L"SoC Reserved (283x)"; > > Sigh, the 2711/2838 naming is very confusing. > >> +  // If we have RAM above the 1 GB mark, declare it >> +  if (SystemMemorySize - SIZE_1GB > 0) { >> +    VirtualMemoryTable[Index].PhysicalBase  = FixedPcdGet64 >> (PcdExtendedMemoryBase); >> +    VirtualMemoryTable[Index].VirtualBase   = >> VirtualMemoryTable[Index].PhysicalBase; >> +    VirtualMemoryTable[Index].Length        = SystemMemorySize - >> SIZE_1GB; >> +    VirtualMemoryTable[Index].Attributes    = >> ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK; >> +    VirtualMemoryInfo[Index].Type           = RPI_MEM_BASIC_REGION; >> +    VirtualMemoryInfo[Index++].Name         = L"Extended System RAM"; >> +  } >> + >>     // End of Table >>     VirtualMemoryTable[Index].PhysicalBase    = 0; >>     VirtualMemoryTable[Index].VirtualBase     = 0; >> > > Reviewed-by: Philippe Mathieu-Daude >