From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 493027803CC for ; Thu, 18 Jan 2024 16:41:06 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=ZIfzIxkTsHThyTcCefFmfF3JC+m441OZ1y/hLGjRu80=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1705596064; v=1; b=JJkfOtGz2jTjbFgfIIbwMHgatmIsK5/k/infnJLCMoyHq1c9Qeg6jxP8P/jJCXHyLzlz65Gd OjQhPYSY3lORHtg9PftI6TQd53+HwmYN4AtIkulBmLZ5dxY3N0rT2mr7R/1C5ssYLPWp6kQWz1W evt83y9oYQoM2S7H7u1AmqbY= X-Received: by 127.0.0.2 with SMTP id BUc7YY7687511xwQUdNkm5a6; Thu, 18 Jan 2024 08:41:04 -0800 X-Received: from mail-qk1-f181.google.com (mail-qk1-f181.google.com [209.85.222.181]) by mx.groups.io with SMTP id smtpd.web11.17429.1705596063852624718 for ; Thu, 18 Jan 2024 08:41:04 -0800 X-Received: by mail-qk1-f181.google.com with SMTP id af79cd13be357-7833b6bb41bso652439585a.3 for ; Thu, 18 Jan 2024 08:41:03 -0800 (PST) X-Gm-Message-State: K8BetMgFRXMGLuGoINDuNWH9x7686176AA= X-Google-Smtp-Source: AGHT+IGsyrIDfgNpU6JVcIStHA67GInL7tcRdCPB4FTo/SLMvvBZSpWvznFZVX1l5nzTIKYFWG58d16B9IQcYhZAoxc= X-Received: by 2002:a0c:e209:0:b0:681:5a26:20c5 with SMTP id q9-20020a0ce209000000b006815a2620c5mr1074716qvl.17.1705596062809; Thu, 18 Jan 2024 08:41:02 -0800 (PST) MIME-Version: 1.0 References: <20240118095018.509362-1-dhaval@rivosinc.com> <20240118095018.509362-2-dhaval@rivosinc.com> In-Reply-To: From: "Dhaval Sharma" Date: Thu, 18 Jan 2024 22:10:51 +0530 Message-ID: Subject: Re: [edk2-devel] [PATCH v1 1/1] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs To: Pedro Falcato Cc: devel@edk2.groups.io, Liming Gao , Michael D Kinney , Zhiguang Liu , Sunil V L , Andrei Warkentin , Laszlo Ersek , Yang Cheng Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,dhaval@rivosinc.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: multipart/alternative; boundary="00000000000074635c060f3b07ce" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=JJkfOtGz; dmarc=none; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io --00000000000074635c060f3b07ce Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Pedro, Agree Assert is slightly more enforcing over logs, but you could still get away with even Assert in release mode. One alternative is to convert VERBOSE into WARNING? =3DD On Thu, Jan 18, 2024 at 9:28=E2=80=AFPM Pedro Falcato wrote: > On Thu, Jan 18, 2024 at 9:50=E2=80=AFAM Dhaval wrot= e: > > > > Some platforms do not implement cache management operations. Especially > > for DMA drivers have code to manage data cache. The code seem to depend > > on the underlying CPU/cache drivers to enact functionality and simply > > return if such functionality is not implemented. However this causes > > issue with CMO implementation which has an assert causing flow to > > hang within debug environment. While it is not an issue in production > > environment there is a recommendation to conver this assert in to > > I don't agree with this patch. As I see it, the library has a simple > contract: Do cache operation X and return. We cannot safely return if > we don't know how to do cache operation X. Say, with a Thead core and > Xtheadcmo. > Any other concerns wrt DMA are, in my view, somewhat separate. > > One can easily theorize a way this change can come to bite us, say, a > storage controller writes bogus data to storage (because the platform > needs explicit cache coherency, and we don't know how to do that) and > causes data corruption. > > > a harmless logger message. Eventually platform/drivers need to have > > better guard for such functionality. > > Like an ASSERT? :) > > -- > Pedro > --=20 Thanks! =3DD -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113999): https://edk2.groups.io/g/devel/message/113999 Mute This Topic: https://groups.io/mt/103805230/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --00000000000074635c060f3b07ce Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Pedro,
Agree Assert is slightly more enforcing over= logs, but you could still get away with even Assert in release mode.
=
One alternative is to convert VERBOSE into WARNING?
=3DD

On Thu, Jan 18, 2024 at 9:28=E2=80=AFPM Pedro Falcato <pedro.falcato@gmail.com&g= t; wrote:
On Thu= , Jan 18, 2024 at 9:50=E2=80=AFAM Dhaval <dhaval@rivosinc.com> wrote:
>
> Some platforms do not implement cache management operations. Especiall= y
> for DMA drivers have code to manage data cache. The code seem to depen= d
> on the underlying CPU/cache drivers to enact functionality and simply<= br> > return if such functionality is not implemented. However this causes > issue with CMO implementation which has an assert causing flow to
> hang within debug environment. While it is not an issue in production<= br> > environment there is a recommendation to conver this assert in to

I don't agree with this patch. As I see it, the library has a simple contract: Do cache operation X and return. We cannot safely return if
we don't know how to do cache operation X. Say, with a Thead core and Xtheadcmo.
Any other concerns wrt DMA are, in my view, somewhat separate.

One can easily theorize a way this change can come to bite us, say, a
storage controller writes bogus data to storage (because the platform
needs explicit cache coherency, and we don't know how to do that) and causes data corruption.

> a harmless logger message. Eventually platform/drivers need to have > better guard for such functionality.

Like an ASSERT? :)

--
Pedro


--
Thanks!
=3DD
_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#113999) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--00000000000074635c060f3b07ce--