From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=LyXERcWt; spf=pass (domain: linaro.org, ip: 209.85.221.66, mailfrom: leif.lindholm@linaro.org) Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by groups.io with SMTP; Fri, 16 Aug 2019 10:03:36 -0700 Received: by mail-wr1-f66.google.com with SMTP id g17so2196778wrr.5 for ; Fri, 16 Aug 2019 10:03:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=31z9272DVDQdXSfgN+KnpAL7jSOD6M7AmOEtqKF1qyE=; b=LyXERcWtMIfDrIsZs/P6FAlFNSpPK6jv7kQgCYxShw4yEmPSe1LYgtK/bXG0LBZ25V byvbXdeYMR6rQB+63H6ugV6eRFgbBcpTnJ904PrjSELw5bCq2ePzoxgN1blnim1Y02D0 GEiFmrwkfTqLlxiXC5HlNi7H2F2UQiwLEJ+FhAbJLe/fu6S7Qz+9K48LovjwQeddiXVR qPOQ35HR2DoK6FYnnUtAQ/vyiJQtCDmH+5GUVS5C3T8O08S5u91JZj/6f9tvO+mOfpjX GevIGIXZG+D8WRSx5/Bmjs2WeBg9FGaJ+yDDqEWUcum/vcZHVSSTq//ToZpjLE6dMev4 kbaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=31z9272DVDQdXSfgN+KnpAL7jSOD6M7AmOEtqKF1qyE=; b=AIQ1TnDq6DPfvRb6k0iQ0eKH80zg5Wc9rQ87yFg21TXfvbSL/0INClK5HrVvT0Pg6q R4fJJdErOK6YHvLGgWj9SUYFIn5RWv9RMCZnRGe/SYx3SDe4NjRiGqo+bdBifRb43PY7 EWOsN++qhZI1aIaFGkag+0I70LL1lqmjJ34ShD8GpGbIrwW7wQTbYo3ZEMwE9kdwqKiS i9lkyCrKu3tBVossSqmRaUbKmxh3FU+FbpG6xmjsm8dQ/oFqvs8T2FET/I63ehJ9aB/V JFO9MLGoEt4at7qYaid2rOGmPVbJyVmLec6SlyFcb0zIDCQi6d1YCE2MxDqPS0GfXqht 6qOw== X-Gm-Message-State: APjAAAXcZD+wXmLrLNY4s0x2yb92pvdwsWfWDQg37RUUiQ4fkkAh0+sZ fI6SN51QgQqe3iSTQhIRrZCwZg== X-Google-Smtp-Source: APXvYqzJqCeSdPx+POrp6jtiRiDYb7zh2KnfoHmgIXraCIUljfB3Ii3+sJfv8pn+b7f1Et72FlA3zQ== X-Received: by 2002:a5d:4946:: with SMTP id r6mr12943948wrs.266.1565975014139; Fri, 16 Aug 2019 10:03:34 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id i5sm6328159wrn.48.2019.08.16.10.03.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 16 Aug 2019 10:03:33 -0700 (PDT) Date: Fri, 16 Aug 2019 18:03:32 +0100 From: "Leif Lindholm" To: Marcin Wojtas Cc: devel@edk2.groups.io, ard.biesheuvel@linaro.org, jsd@semihalf.com, jaz@semihalf.com, kostap@marvell.com Subject: Re: [edk2-platforms: PATCH v2 01/10] Marvell/Armada7k8k: Fix 32-bit compilation Message-ID: <20190816170332.GT29255@bivouac.eciton.net> References: <1565837654-13258-1-git-send-email-mw@semihalf.com> <1565837654-13258-2-git-send-email-mw@semihalf.com> MIME-Version: 1.0 In-Reply-To: <1565837654-13258-2-git-send-email-mw@semihalf.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Aug 15, 2019 at 04:54:05AM +0200, Marcin Wojtas wrote: > It turned out, that the recently added features broke > ARM compilation. Fix all issues: > * Use SIGNATURE_32 only > * Do not shift address by 32-bit in ICU > * Limit memory for ARM build to 1GB and stop using non-existent PCD > > Signed-off-by: Marcin Wojtas > --- > Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.h | 2 +- > Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.h | 2 +- > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c | 4 ++++ > Silicon/Marvell/Library/IcuLib/IcuLib.c | 7 ++++++- > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/ARM/ArmPlatformHelper.S | 11 ----------- > 5 files changed, 12 insertions(+), 14 deletions(-) > > diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.h b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.h > index a6f551b..7ecb4e1 100644 > --- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.h > +++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.h > @@ -18,7 +18,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #include > > -#define MV_BOARD_DESC_SIGNATURE SIGNATURE_64 ('M', 'V', 'B', 'R', 'D', 'D', 'S', 'C') > +#define MV_BOARD_DESC_SIGNATURE SIGNATURE_32 ('B', 'D', 'S', 'C') > > typedef struct { > MARVELL_BOARD_DESC_PROTOCOL BoardDescProtocol; > diff --git a/Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.h b/Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.h > index 1cb006a..600d5db 100644 > --- a/Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.h > +++ b/Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.h > @@ -23,7 +23,7 @@ > > #include > > -#define MV_GPIO_SIGNATURE SIGNATURE_64 ('M', 'V','_','G', 'P', 'I','O',' ') > +#define MV_GPIO_SIGNATURE SIGNATURE_32 ('G', 'P', 'I', 'O') Can you epxlain why you opted to change the size of the signature instead of the signature field in the structs? I'm not super happy about this, since we still have UINTN signature, not UINT32. Since the signature is *not* architecture dependent, it should have a fixed size. > > // Marvell MV_GPIO Controller Registers > #define MV_GPIO_DATA_OUT_REG (0x0) > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > index a735fe5..5ff6bb1 100644 > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > @@ -36,6 +36,7 @@ GetDramSize ( > IN OUT UINT64 *MemSize > ) > { > +#if defined(MDE_CPU_AARCH64) > ARM_SMC_ARGS SmcRegs = {0}; > EFI_STATUS Status; > > @@ -48,6 +49,9 @@ GetDramSize ( > ArmCallSmc (&SmcRegs); > > *MemSize = SmcRegs.Arg0; > +#else > + *MemSize = FixedPcdGet64 (PcdSystemMemorySize); > +#endif Can you add a comment explaining why a 32-bit SMC call is not possible? > > return EFI_SUCCESS; > } > diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.c b/Silicon/Marvell/Library/IcuLib/IcuLib.c > index 343c21b..422388c 100644 > --- a/Silicon/Marvell/Library/IcuLib/IcuLib.c > +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.c > @@ -185,10 +185,15 @@ IcuConfigure ( > for (Index = 0; Index < IcuGroupMax; Index++, Msi++) { > MmioWrite32 (IcuBase + ICU_SET_SPI_AL (Msi->Group), > Msi->SetSpiAddr & 0xFFFFFFFF); > - MmioWrite32 (IcuBase + ICU_SET_SPI_AH (Msi->Group), Msi->SetSpiAddr >> 32); > MmioWrite32 (IcuBase + ICU_CLR_SPI_AL (Msi->Group), > Msi->ClrSpiAddr & 0xFFFFFFFF); > +#if defined(MDE_CPU_AARCH64) > + MmioWrite32 (IcuBase + ICU_SET_SPI_AH (Msi->Group), Msi->SetSpiAddr >> 32); > MmioWrite32 (IcuBase + ICU_CLR_SPI_AH (Msi->Group), Msi->ClrSpiAddr >> 32); > +#else > + MmioWrite32 (IcuBase + ICU_SET_SPI_AH (Msi->Group), 0); > + MmioWrite32 (IcuBase + ICU_CLR_SPI_AH (Msi->Group), 0); > +#endif I don't understand what's going on here - add a comment? Best Regards, Leif > } > > /* Mask all ICU interrupts */ > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/ARM/ArmPlatformHelper.S b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/ARM/ArmPlatformHelper.S > index 4416163..db43b0f 100644 > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/ARM/ArmPlatformHelper.S > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/ARM/ArmPlatformHelper.S > @@ -28,17 +28,6 @@ ASM_FUNC(ArmPlatformPeiBootAction) > .err PcdSystemMemoryBase should be 0x0 on this platform! > .endif > > - .if FixedPcdGet64 (PcdSystemMemorySize) > FixedPcdGet32 (PcdDramRemapTarget) > - // > - // Use the low range for UEFI itself. The remaining memory will be mapped > - // and added to the GCD map later. > - // > - ADRL (r0, mSystemMemoryEnd) > - MOV32 (r2, FixedPcdGet32 (PcdDramRemapTarget) - 1) > - mov r3, #0 > - strd r2, r3, [r0] > - .endif > - > bx lr > > //UINTN > -- > 2.7.4 >