linux-kernel - Re: [PATCH v3 0/3] Add qcom hvc/shmem transport

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 7 Sep 2023 11:36:19 +0100
From:   Sudeep Holla <sudeep.holla@....com>
To:     Nikunj Kela <quic_nkela@...cinc.com>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        cristian.marussi@....com, Sudeep Holla <sudeep.holla@....com>,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        conor+dt@...nel.org, agross@...nel.org, andersson@...nel.org,
        konrad.dybcio@...aro.org, linux-arm-kernel@...ts.infradead.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v3 0/3] Add qcom hvc/shmem transport

On Tue, Sep 05, 2023 at 06:37:14PM +0200, Krzysztof Kozlowski wrote:
> On 05/09/2023 18:06, Nikunj Kela wrote:
> > 
> > On 8/11/2023 10:57 AM, Nikunj Kela wrote:
> >> This change introduce a new transport channel for Qualcomm virtual
> >> platforms. The transport is mechanically similar to ARM_SCMI_TRANSPORT_SMC.
> >> The difference between the two transports is that a parameter is passed in
> >> the hypervisor call to identify which doorbell to assert. This parameter is
> >> dynamically generated at runtime on the device and insuitable to pass via
> >> the devicetree.
> >>
> >> The function ID and parameter are stored by firmware in the shmem region.
> >>
> >> This has been tested on ARM64 virtual Qualcomm platform.
> >>
> >> ---
> >> v3 -> fix the compilation error reported by the test bot,
> >>        add support for polling based instances
> >>
> >> v2 -> use allOf construct in dtb schema,
> >>        remove wrappers from mutexes,
> >>        use architecture independent channel layout
> >>
> >> v1 -> original patches
> >>
> >> Nikunj Kela (3):
> >>    dt-bindings: arm: convert nested if-else construct to allOf
> >>    dt-bindings: arm: Add qcom specific hvc transport for SCMI
> >>    firmware: arm_scmi: Add qcom hvc/shmem transport
> >>
> >>   .../bindings/firmware/arm,scmi.yaml           |  67 ++---
> >>   drivers/firmware/arm_scmi/Kconfig             |  13 +
> >>   drivers/firmware/arm_scmi/Makefile            |   2 +
> >>   drivers/firmware/arm_scmi/common.h            |   3 +
> >>   drivers/firmware/arm_scmi/driver.c            |   4 +
> >>   drivers/firmware/arm_scmi/qcom_hvc.c          | 232 ++++++++++++++++++
> >>   6 files changed, 293 insertions(+), 28 deletions(-)
> >>   create mode 100644 drivers/firmware/arm_scmi/qcom_hvc.c
> > Gentle Ping!

Pong !

>
> It's third ping these two weeks from Qualcomm. Folks, it is merge
> window. What do you think will happen with your ping during this time?
>

+1

Okay, here is the deal with this patch set. As you are aware that a previous
merged solution was abandoned by Qcom in a single kernel release cycle. So
I decided to ignore this for one or 2 kernel release cycle to make sure
Qcom makes up their mind on the design and then we can see how to proceed.
Qcom must understand upstream kernel is not a playground to push their
design which they might decided to drop support for in such short period.
Please understand the upstream kernel supports platforms that are more than
few decades old. It is not like the mobile platforms that are hardly supported
for couple of years. And similarly, we push core support if and only if we
know for sure it will be used on some platform. I trusted Qcom with the
previous extension of SMC/HVC transport but I was proven wrong.

Also, I definitely don't like the way you have copied the whole smc.c
and changed it to Qcom's need and made it qcom_hvc.c. Just add the required
changes in smc.c.

--
Regards,
Sudeep