Message-ID: <20230705-patio-daringly-7fdb62228bc0@spud>
Date:   Wed, 5 Jul 2023 16:33:25 +0100
From:   Conor Dooley <conor@...nel.org>
To:     Palmer Dabbelt <palmer@...belt.com>
Cc:     cuiyunhui@...edance.com, Ard Biesheuvel <ardb@...nel.org>,
        sunilvl@...tanamicro.com, Paul Walmsley <paul.walmsley@...ive.com>,
        aou@...s.berkeley.edu, linux-riscv@...ts.infradead.org,
        rminnich@...il.com, Mark Rutland <mark.rutland@....com>,
        lpieralisi@...nel.org, rafael@...nel.org, lenb@...nel.org,
        jdelvare@...e.com, yc.hung@...iatek.com,
        angelogioacchino.delregno@...labora.com,
        allen-kh.cheng@...iatek.com, pierre-louis.bossart@...ux.intel.com,
        tinghan.shen@...iatek.com, linux-kernel@...r.kernel.org,
        linux-acpi@...r.kernel.org, geshijian@...edance.com,
        weidong.wd@...edance.com
Subject: Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

Hey,

On Wed, Jul 05, 2023 at 07:17:29AM -0700, Palmer Dabbelt wrote:
> On Wed, 05 Jul 2023 04:42:47 PDT (-0700), cuiyunhui@...edance.com wrote:
> > Here's version 3 of patch series.
> > 
> > V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
> > consensus with the Maintainers.
> > Please refer to:
> > https://patches.linaro.org/project/linux-acpi/patch/20230426034001.16-1-cuiyunhui@bytedance.com/
> 
> From looking at that thread it seems that the consensus is this is a bad
> idea?  Sorry if I'm just missing something...

"consensus" meaning that Ard told them what he was and was not prepared
to accept in common code, and left the decision on what he was not up to
the RISC-V maintainers.

While this version of the series seems to address some of my general
code comments on the v2 (although I have not yet looked more than skin
deep), there are some outstanding, higher level, questions that were
asked on v2 that do not seem to have been answered satisfactorily yet:

- "So, could you please bring this topic for discussion in on the riscv
  tech-brs mailing list (https://lists.riscv.org/g/tech-brs) and get
  agreement?" Sunil has asked this as RVI specs have an interest in
  cross-os booting contracts. See:
  https://lore.kernel.org/linux-riscv/CAEEQ3wnsedWJYEEg8z+3C_HuCca0nD50NGpCdU3scxavrrOucA@mail.gmail.com/

- "I am curious how do you handle EFI memory map dependencies." to
  which the answer was "a memory node in DTS can solve it."
  I don't see anything in this version of the patchset that actually
  reads a DTS node when ACPI is enabled. If I am missing some codepath
  that does this, please let point it out. See:
  https://lore.kernel.org/linux-riscv/CAEEQ3wnsedWJYEEg8z+3C_HuCca0nD50NGpCdU3scxavrrOucA@mail.gmail.com/

- "I'm not a big fan of adding yet another interface. Have you considered
  doing something like [1]?" where [1] was:
  https://github.com/tianocore/tianocore.github.io/wiki/UefiPayloadPkg
  The response to this question was "This has been discussed many times
  with Ard, Please refer to <v1>". I don't see how this answers the
  question to be honest & Andrew's follow-up question asking for
  clarification went unanswered:
  https://lore.kernel.org/linux-riscv/20230703-6ac90a2de15f1017bc0ced74@orel/
  Jess, Emil and Bjorn have all also commented about you could load a
  small EFI payload from Coreboot. I don't see any responses to those
  questions.

运辉崔, please try to address all outstanding comments (and give people
a chance to reply to you) before sending new versions.

Cheers,
Conor.

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists