From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f53.google.com (mail-wr1-f53.google.com [209.85.221.53]) by mx.groups.io with SMTP id smtpd.web12.9767.1589886955355360372 for ; Tue, 19 May 2020 04:15:55 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=SBrzSM/T; spf=pass (domain: nuviainc.com, ip: 209.85.221.53, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f53.google.com with SMTP id h17so15410832wrc.8 for ; Tue, 19 May 2020 04:15:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=HmHRCodDvhpgtfBFzHpXXJILKwSTJo38rMrzDtb/eE0=; b=SBrzSM/T7Mg1xwXhlVFdlbwp5Yu2dPROQdAYS+Wr3oYvZwP0+z251qKQOALYLQ9R9m 0i9nVE6AJoREcmABF3lnHyCnYofjFi2XLMc5nrXg0ibJJ6YrJQpa05ogM5GhdNW0nk29 q5FfsmLPejIV3Oo/8mMEZCyftkyKIWH42LnXYOFI/0br/RFO9gLmA8qr02pjq8v3i1C4 AqtjYKMAXu4pXO5hpeyiUEFmCrIW2o1JhmmM79saUp/ntOrmkF2rEnS6uFM8zDJsivwE 3D4ZpEpQLtIiDmpgvK9bQWM9PZCe5b1tuJ5pj+Np17wREWQI7bKG5oAGfnszd4/J5kxh P1JA== 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=HmHRCodDvhpgtfBFzHpXXJILKwSTJo38rMrzDtb/eE0=; b=HKGRR5YI/yZFK5dO2N6Aq9P5pNPM6aIWkkPpbtEVVBYhkXCBQuqTIrWvxEQe7j7Qtt EC07m2pHeOVZW4U7Fwfwi9Tr+4Nk92gKD1u9PPSMexK63a2mBPo1IveIgpP9bH9i5Ra4 0AJq5BljfEuLK4Uc20TBk97Ti/rwyswxTkU9E4EWyTMZEmDU72rKt8MXK8DllruLhVuN XMsSQ78Jc+DwuXM4JQJrA8KhSXouqCCTQMf64rI4FAucXdfK79Xs6WiqTTt1JdiGxrf3 ReA6QZFmszvMkH2qfMMjaVb1Ptdmh/25eRACl37PENhhhgU4sAXF3c+3sHskU3wYzRjk 20HQ== X-Gm-Message-State: AOAM530SOoTih/c8HD+VPU38fyODA9Sq9m6xESzblfojGZXEycetcbDP kcYR8xkRbLvj4BdvA8CmzjrjJ8F/JK0l5we77FBmu3kx2qVPprgbz5BTPP/vhicgz06anq2StA7 MhZ/o30WitzC7GzuG9uO6F4UutLPsx/mqxcce3nsxfznCPWUvaDX2qOTRqotcjkYIXA== X-Google-Smtp-Source: ABdhPJxzCSwPfmN71ta4i0J9k1at4Tvhn3fmFE0JAFrQkq4kxtUsWYKkQdZRNA6d/IRRW3+QGGh0cg== X-Received: by 2002:adf:f981:: with SMTP id f1mr24875328wrr.244.1589886953535; Tue, 19 May 2020 04:15:53 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id a8sm21076051wrg.85.2020.05.19.04.15.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 May 2020 04:15:52 -0700 (PDT) Date: Tue, 19 May 2020 12:15:50 +0100 From: "Leif Lindholm" To: devel@edk2.groups.io, lersek@redhat.com Cc: ard.biesheuvel@arm.com, graeme.gregory@linaro.org, tanmay.jagdale@linaro.org, Sami Mujawar Subject: Re: [edk2-devel] [PATCH] ArmPkg/PlatformBootManagerLib: reject 'default' parity and stop bit count Message-ID: <20200519111550.GM10467@vanye> References: <20200518171148.6113-1-ard.biesheuvel@arm.com> <6f410b08-f868-8b38-60fd-2b863a89658e@redhat.com> MIME-Version: 1.0 In-Reply-To: <6f410b08-f868-8b38-60fd-2b863a89658e@redhat.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline +Sami On Tue, May 19, 2020 at 12:00:02 +0200, Laszlo Ersek wrote: > On 05/18/20 19:11, Ard Biesheuvel wrote: > > In the ArmPkg version of PlatformBootManagerLib, we construct a > > serial device path based on the default settings for baud rate, > > parity and the number of stop bits, to ensure that a serial console > > is available even on the very first boot. > > > > This assumes that PcdUartDefaultParity or PcdUartDefaultStopBits are > > not set to '0', meaning 'the default', as there is no default for > > these when constructing a device path. > > > > So add a couple of ASSERT()s to make sure that we catch this condition, > > since it otherwise ignores the bogus device path silently, which is > > rather tedious to debug,. > > > > Signed-off-by: Ard Biesheuvel > > --- > > ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > index e6e788e0f107..a030d510aa62 100644 > > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > @@ -583,6 +583,8 @@ PlatformBootManagerBeforeConsole ( > > // > > // Add the hardcoded serial console device path to ConIn, ConOut, ErrOut. > > // > > + ASSERT (FixedPcdGet8 (PcdUartDefaultParity) > 0); > > + ASSERT (FixedPcdGet8 (PcdUartDefaultStopBits) > 0); > > ASSERT (FixedPcdGet8 (PcdDefaultTerminalType) == 4); > > CopyGuid (&mSerialConsole.TermType.Guid, &gEfiTtyTermGuid); > > > > > > Given that these are fixed PCDs, I'd recommend STATIC_ASSERT(). We've > recently put STATIC_ASSERT() to use (in "OvmfPkg/MptScsiDxe/MptScsi.c") > in combination with a fixed PCD: > > STATIC_ASSERT ( > FixedPcdGet8 (PcdMptScsiMaxTargetLimit) < 255, > "Req supports 255 targets only (max target is 254)" > ); > > Just an idea of course; if that's out of scope for now, I have nothing > against this patch. That sounds useful, but should then probably get applied to the TerminalType as well, so could come in as a separate patch on top. Sami: could this be an alternative resolution for https://edk2.groups.io/g/devel/message/59740 ? Regards, Leif > Thanks, > Laszlo > > > >