From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) by mx.groups.io with SMTP id smtpd.web08.471.1665422454656893148 for ; Mon, 10 Oct 2022 10:20:54 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@ventanamicro.com header.s=google header.b=I1LC3Ns9; spf=pass (domain: ventanamicro.com, ip: 209.85.214.177, mailfrom: sunilvl@ventanamicro.com) Received: by mail-pl1-f177.google.com with SMTP id f23so10957086plr.6 for ; Mon, 10 Oct 2022 10:20:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=znWJCWwaFPnBkkKpUHfB4m0dNiHr7Hpc+/SOD43h0sU=; b=I1LC3Ns9A4sy+MHTzDBOXA61pE+FjQhuUc9XIrhw8TJngtzZyx+nwAOOMZaanQ49mH uC5lZeR50cBEoPZ+/OS2F71K8Ye5ackes+7xKfOQoZ2fpc8hfRFanhMwC7q2hAV32fYy CTXIX3v1srw8Zx3JTfsOHr7HOCcXijDLYVKkgfIwCstrEZ21bNXhDy+ww8MYLbQy4L/c FHlzfyH3ZfsgL7MjlhXLHim2gkHRIQG4PlMtKUS1/+y2I0Ltr9cmToN/i+LObQH0YkVD mR3YZTNJAPfA06lVEX5fV8+d7wtYrRc3TwmYOSrPVavliVg2I7rNLLMx46Fn43Z1SwXu 8H1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=znWJCWwaFPnBkkKpUHfB4m0dNiHr7Hpc+/SOD43h0sU=; b=VrKyZuBBGZ8riRUoTMPc4XGTvK5KxL/SoYlvqJSsQVdt3YxjCY1CqNT7+VCO72M1gc frArl6khxbDPKkpyJSpESfymuPe/4rOBJVKtDZlPbCJLwW28vx9V2GU8o4qFm9eQPeKt FLElV2cAD28D9G+QIi1MYzIQPPU2DCStaptSY5b1+x65uEILLY6ZTY2KoNm0x+pQ6vVt EEXGqd6uWjAKJicesS3lrD9qIPwbhAJNWXZr6RgD9A8YXSr3op7vOawa1Mqb3GxNSKIx q4fgFCegUr24UqJg4uzf3dGq/3L11TKvegI32pjXywO1JlwxUCSJkFhRK/19vi51WoOH NZTw== X-Gm-Message-State: ACrzQf2rnPxHJMlkT3Dd+CIDbbR+DyHGdMMQUL2L/JqX+XCyvZmQ3smj V465m4HrgZjivtegDIydUvB/gw== X-Google-Smtp-Source: AMsMyM69TyZo9PNZIm6eSg4LvlZ/CKHV/0MpA+hRGykcXzqJR6yCyl1ggP8Gngau7apiikyi22OUcw== X-Received: by 2002:a17:902:a611:b0:178:6b71:2ee5 with SMTP id u17-20020a170902a61100b001786b712ee5mr19939365plq.53.1665422454150; Mon, 10 Oct 2022 10:20:54 -0700 (PDT) Return-Path: Received: from sunil-laptop ([49.206.13.138]) by smtp.gmail.com with ESMTPSA id i20-20020a63e914000000b00434272fe870sm6561620pgh.88.2022.10.10.10.20.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 10 Oct 2022 10:20:53 -0700 (PDT) Date: Mon, 10 Oct 2022 22:50:48 +0530 From: "Sunil V L" To: Ard Biesheuvel Cc: devel@edk2.groups.io, Ard Biesheuvel , Jiewen Yao , Jordan Justen , Gerd Hoffmann Subject: Re: [edk2-staging/RiscV64QemuVirt PATCH 26/29] OvmfPkg: Add generic Qemu NOR flash DXE driver Message-ID: References: <20221010101202.1146624-1-sunilvl@ventanamicro.com> <20221010101202.1146624-27-sunilvl@ventanamicro.com> MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Oct 10, 2022 at 06:16:28PM +0200, Ard Biesheuvel wrote: > On Mon, 10 Oct 2022 at 18:05, Sunil V L wrote: > > > > On Mon, Oct 10, 2022 at 05:29:15PM +0200, Ard Biesheuvel wrote: > > > On Mon, 10 Oct 2022 at 17:19, Sunil V L wrote: > > > > > > > > On Mon, Oct 10, 2022 at 12:39:21PM +0200, Ard Biesheuvel wrote: > > > > > On Mon, 10 Oct 2022 at 12:13, Sunil V L wrote: > > > > > > > > > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4076 > > > > > > > > > > > > RISC-V needs NorFlashDxe driver for qemu virt machine. The > > > > > > ArmPlatformPkg has this driver but migrating it to generic > > > > > > package like MdeModulePkg introduces circular dependencies. > > > > > > So, add a simplified version of the NorFlashDxe driver in > > > > > > OvmfPkg. > > > > > > > > > > > > > > > > So what is the difference between this simplified version and the old > > > > > version? And it is backed in QEMU by the same NOR flash emulation, > > > > > shouldn't we use this driver for ArmVirtQemu as well? > > > > > > > > I agree. If we can break the dependency on EmbeddedPkg due to > > > > NvVarStoreFormattedLib, then we can migrate to MdeModulePkg and all > > > > consumers can use the same driver which would be the best solution IMO. > > > > > > > > Could you please let me know why NvVarStoreFormattedLib is added > > > > in EmbeddedPkg instead of MdePkg or MdeModulePkg? Is it only for > > > > non-server class platforms? I don't see it doing much so not sure > > > > its use case. > > > > > > > > > > I think that library as well as the definition of > > > gEdkiiNvVarStoreFormattedGuid should be moved to MdeModulePkg. > > > > > > Then, we can look at moving NorFlashDxe in there as well. > > > > Great. Let me rework these patches then. > > > > > > But you haven't answered my question regarding how your version was simplified. > > > > > > > Oh sorry. I forgot to modify the commit message but what I really meant > > was removing the code which depends upon PcdNorFlashCheckBlockLocked for > > virtual platforms. I thought it is useful only for real platforms and > > qemu doesn't emulate it. Let me know if I am wrong. > > Since the driver is copied for OVMF, I didn't want to add the PCD unnecessarily. > > > > What I would like to do is the following: > > - move NvVarstoreFormattedLib into MdeModulePkg (as you proposed already) > - create a new NorFlashDxe in Ovmf that is cleaned up, drops the > handling of locked blocks (as you suggest) and minimizes the number of > transitions between array mode and programming mode (i have some > patches i can share as a starting point) > - move ArmPlatformPkg's NorFlashDxe into its only remaining user, > which is is Platform/ARM in edk2-platforms (there are two more users > in edk2-platforms, but both are QEMU based on so they can switch to > the OVMF one) Interesting. In that case, we can continue with current approach of copy in Ovmf and add the optimization. I was assuming you want the same driver for real and virtual platforms in MdeModulePkg :-). Sure, please share those optimization changes. I will create patches as you have outlined here. > > > > > Note that there is some room for improvement in that driver in > > > relation to execution under KVM: switching between programming mode > > > and array mode involves setting up/tearing down the KVM memslot, and > > > currently, the driver is far from optimized when it comes to > > > minimizing the number of transitions between read mode and write mode. > > > > I was not looking at optimizing it at this point of time. > > > > I understand. And yet you are already proposing a different version of > NorFlashDxe, and I don't want to clone the existing NorFlashDxe > without cleaning it up first. Understood. Thanks for your help. Thanks Sunil