From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lf1-f51.google.com (mail-lf1-f51.google.com [209.85.167.51]) by mx.groups.io with SMTP id smtpd.web11.7923.1680874154234127880 for ; Fri, 07 Apr 2023 06:29:14 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=J26X84zl; spf=pass (domain: linaro.org, ip: 209.85.167.51, mailfrom: marcin.juszkiewicz@linaro.org) Received: by mail-lf1-f51.google.com with SMTP id z8so218188lfb.12 for ; Fri, 07 Apr 2023 06:29:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1680874152; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=/294Ar8fPKiJgMqGw5EVCKfO/oBKwfxoTmvIcxbJ+U4=; b=J26X84zlYNAbeSDizaboszdTt7m/mh04onPIfuzfVVBXVRglaEtsTuj9CUDMSFLI+8 bNFlXE1PDxMNeTCU2DiSUEw2oXjYikiEpni5deQh8YbEQYb97jpUjqvjz7IIyf6G61TX xMW4mnxKttTi/H8PL/m4v2MLTCkRXH/CNEbTZC8xtKZxgw9GCIYn1WV+qAy8aVEfbXaO NeKjFP5hsNFZ4SWxCi268M+tFeU1jWlI9il8Nplv8wm7jxF86h/M9ARPNS7X8XH9leJL 4y5KtD1LhyAS6j/0BOzHdOKmHMjZZoLZojhYw3OW6HkZkFwKXlvM+B3EUzvdXHI359Tn M1EA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680874152; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=/294Ar8fPKiJgMqGw5EVCKfO/oBKwfxoTmvIcxbJ+U4=; b=obcYdGoegfYdLTqZBYgTx+0bg5W1dIjfhbpMK34AY8Ow+N7ja8viSRMSq+OqEkRXfp qJWI+TGlnlg+S2L34ApLgJhDxWYOEkCHaKl3yxFasV7axknsO750gg5Z7SfA6n530YPx D6n/Nyy2qlwMbKTo+6ZEL0zzE+EQVzRT9rn90C124+tCWWLEk/zgPvSO4IKGckl1BoZd wTvUOrEzffrv82tJQtQwYh0fiUvctNx7LA4FhP/yUzFChKCQutMTiLjR0HnVxJl/9E9X DB58VERLRIdMbtA4ZW/2KmSCfVjSgqutpxq9E3ShUAtHqX0oGHNijh0hjKB6Pksu2Tof WjNw== X-Gm-Message-State: AAQBX9dzy+lOOQZSiCHC4kfs1ZTuc4lEu26ToPBsTK84Q0FdnfvSiG/2 pQzS8WS/R6XIory8JlbyIPfRttzh6gbiz4U6YVFPZQ== X-Google-Smtp-Source: AKy350Y7mYfFgxOX35+MDhDAI6rHElX2i+SC2Y0XbW4oivIQPN8dL94SJJ6sbujWUPR43/JOj5OULQ== X-Received: by 2002:ac2:5197:0:b0:4dd:cb1d:b3cc with SMTP id u23-20020ac25197000000b004ddcb1db3ccmr677549lfi.11.1680874152059; Fri, 07 Apr 2023 06:29:12 -0700 (PDT) Return-Path: Received: from [192.168.200.206] (83.11.21.111.ipv4.supernova.orange.pl. [83.11.21.111]) by smtp.gmail.com with ESMTPSA id u13-20020ac251cd000000b004eb4074f40fsm707295lfm.241.2023.04.07.06.29.11 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 07 Apr 2023 06:29:11 -0700 (PDT) Message-ID: <1fff4ffa-6819-fb7f-4d81-3237b0dab7e8@linaro.org> Date: Fri, 7 Apr 2023 15:29:10 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.1 Subject: Re: [edk2-devel] [PATCH v3] add ArmCpuInfo EFI application To: devel@edk2.groups.io References: <20230407124731.121236-1-marcin.juszkiewicz@linaro.org> From: "Marcin Juszkiewicz" Organization: Linaro In-Reply-To: Content-Language: pl-PL, en-GB, en-HK Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit W dniu 7.04.2023 o 15:02, Pedro Falcato pisze: >> +EFI_STATUS >> +EFIAPI >> +UefiMain ( >> + IN EFI_HANDLE ImageHandle, >> + IN EFI_SYSTEM_TABLE *SystemTable >> + ) >> +{ >> + UINT64 aa64dfr0_el1 = read_aa64dfr0_el1 (); >> + UINT64 aa64dfr1_el1 = read_aa64dfr1_el1 (); >> + UINT64 aa64isar0_el1 = read_aa64isar0_el1 (); >> + UINT64 aa64isar1_el1 = read_aa64isar1_el1 (); >> + UINT64 aa64isar2_el1 = read_aa64isar2_el1 (); >> + UINT64 aa64mmfr0_el1 = read_aa64mmfr0_el1 (); >> + UINT64 aa64mmfr1_el1 = read_aa64mmfr1_el1 (); >> + UINT64 aa64mmfr2_el1 = read_aa64mmfr2_el1 (); >> + UINT64 aa64pfr0_el1 = read_aa64pfr0_el1 (); >> + UINT64 aa64pfr1_el1 = read_aa64pfr1_el1 (); > > EDK2 requires separation between declarations and code (something > alike old C89 semantics, but stricter). so: > UINT64 aa64pfr1_el1; > <...> > aa64pfr1_el1 = read_aa64pfr1_el1 (); done >> + >> + /* UINT64 aa64smfr0_el1 = read_aa64smfr0_el1 ();*/ >> + /* UINT64 aa64zfr0_el1 = read_aa64zfr0_el1 ();*/ > > Dead Code? I ignore SVE/SME registers for now. Dropped code. >> + >> + AsciiPrint ("ID_AA64MMFR0_EL1 = 0x%016lx\n", aa64mmfr0_el1); >> + AsciiPrint ("ID_AA64MMFR1_EL1 = 0x%016lx\n", aa64mmfr1_el1); >> + AsciiPrint ("ID_AA64MMFR2_EL1 = 0x%016lx\n", aa64mmfr2_el1); >> + AsciiPrint ("ID_AA64PFR0_EL1 = 0x%016lx\n", aa64pfr0_el1); >> + AsciiPrint ("ID_AA64PFR1_EL1 = 0x%016lx\n", aa64pfr1_el1); >> + AsciiPrint ("ID_AA64ISAR0_EL1 = 0x%016lx\n", aa64isar0_el1); >> + AsciiPrint ("ID_AA64ISAR1_EL1 = 0x%016lx\n", aa64isar1_el1); >> + AsciiPrint ("ID_AA64ISAR2_EL1 = 0x%016lx\n", aa64isar2_el1); >> + AsciiPrint ("ID_AA64DFR0_EL1 = 0x%016lx\n", aa64dfr0_el1); >> + AsciiPrint ("ID_AA64DFR1_EL1 = 0x%016lx\n", aa64dfr1_el1); // ignore > > Why ignore? Was 0 on systems I checked. Dropped. >> + /* AsciiPrint ("ID_AA64SMFR0_EL1 = 0x%016lx\n", aa64smfr0_el1);*/ >> + /* AsciiPrint ("ID_AA64ZFR0_EL1 = 0x%016lx\n", aa64zfr0_el1);*/ > > dead? Dropped. >> diff --git a/ArmPkg/Application/ArmCpuInfo/readregs.s b/ArmPkg/Application/ArmCpuInfo/readregs.s > > ASM files that require preprocessing should have a capital S here (.S vs .s) After renaming it to readregs.S (and changing in ArmCpuInfo.inf) I get: build.py... /home/marcin/devel/linaro/sbsa-qemu/code/edk2/ArmPkg/Application/ArmCpuInfo/ArmCpuInfo.inf(29): error 000E: File/directory not found in workspace readregs.S is not found in packages path: >> new file mode 100644 >> index 000000000000..052834b3c3f2 >> --- /dev/null >> +++ b/ArmPkg/Application/ArmCpuInfo/readregs.s >> @@ -0,0 +1,49 @@ >> +#include >> + >> +ASM_FUNC(read_aa64pfr0_el1) >> + mrs x0, ID_AA64PFR0_EL1; >> + ret; > > ASM lines (for GAS at least) don't usually end in semicolons. so > +ASM_FUNC(read_aa64pfr0_el1) > + mrs x0, ID_AA64PFR0_EL1 > + ret > > You'd only need the semicolons if you had multiple instructions in one > line (like mrs x0, ID_AA...; ret) thanks, dropped semicolons >> +# ASM_FUNC(read_aa64zfr0_el1) >> +# mrs x0, ID_AA64ZFR0_EL1; >> +# ret; >> + >> +# ASM_FUNC(read_aa64smfr0_el1) >> +# mrs x0, ID_AA64SMFR0_EL1; >> +# ret; > > Dead code? Those two registers names were missing so got disabled. Dropped. > Brief comments in general: > 1) You use binary literals extensively, which are not portable (GNU C > extension, AFAIK not in MSVC) My C skills are rusty and the last time I used compiler other than GCC was in Borland Turbo C 3 times. > 2) Naming of identifiers (vars, functions, etc) needs to follow the > EDK2 style. eg: > description vs Description > read_aa64mmfr1_el1 vs ReadAa64MmfrEl1 > etc... ok > Forgot mentioning: STATIC vs static, CONST vs const, > VOID vs void, CHAR8 vs char, etc. > All fun microsoftisms you need to use here. Will be fun.