| 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
| ||
|
Message-ID: <CAOdFzcj0qqercZrcgabwU2opsKsEo2OnEgOQA=VGrefCdrT9bA@mail.gmail.com>
Date: Tue, 28 Oct 2025 17:44:07 -0700
From: Christopher Lew <christopher.lew@....qualcomm.com>
To: Bjorn Andersson <andersson@...nel.org>
Cc: Jingyi Wang <jingyi.wang@....qualcomm.com>,
Konrad Dybcio <konradybcio@...nel.org>, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org, Chris Lew <chris.lew@....qualcomm.com>
Subject: Re: [PATCH 2/2] soc: qcom: smp2p: Add support for smp2p v2
On Wed, Oct 22, 2025 at 3:16 PM Bjorn Andersson <andersson@...nel.org> wrote:
>
> On Tue, Sep 23, 2025 at 09:18:43PM -0700, Jingyi Wang wrote:
> > From: Chris Lew <chris.lew@....qualcomm.com>
> >
> > Some remoteproc need smp2p v2 support, mirror the version written by the
> > remote if the remote supports v2. This is needed if the remote does not
> > have backwards compatibility with v1. And reset entry last value on SSR for
> > smp2p v2.
> >
> > Signed-off-by: Chris Lew <chris.lew@....qualcomm.com>
> > Co-developed-by: Jingyi Wang <jingyi.wang@....qualcomm.com>
> > Signed-off-by: Jingyi Wang <jingyi.wang@....qualcomm.com>
>
> Please confirm that you really co-developed (pair programming) this
> patch with Chris.
>
> Isn't this a patch from Chris, that you're "forwarding", i.e. both
> Signed-off-by should be there, but the Co-developed-by shouldn't.
>
> > ---
> > drivers/soc/qcom/smp2p.c | 35 ++++++++++++++++++++++++++++++++---
> > 1 file changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
> > index e2cfd9ec8875..5ea64a83c678 100644
> > --- a/drivers/soc/qcom/smp2p.c
> > +++ b/drivers/soc/qcom/smp2p.c
> > @@ -48,10 +48,13 @@
> > #define SMP2P_MAGIC 0x504d5324
> > #define SMP2P_ALL_FEATURES SMP2P_FEATURE_SSR_ACK
> >
> > +#define SMP2P_VERSION_1 1
> > +#define SMP2P_VERSION_2 2
>
> #define ONE 1
> #define TWO 2
>
> #define PLEASE_DONT true
>
> > +
> > /**
> > * struct smp2p_smem_item - in memory communication structure
> > * @magic: magic number
> > - * @version: version - must be 1
> > + * @version: version
> > * @features: features flag - currently unused
> > * @local_pid: processor id of sending end
> > * @remote_pid: processor id of receiving end
> > @@ -180,14 +183,23 @@ static void qcom_smp2p_kick(struct qcom_smp2p *smp2p)
> > static bool qcom_smp2p_check_ssr(struct qcom_smp2p *smp2p)
> > {
> > struct smp2p_smem_item *in = smp2p->in;
> > + struct smp2p_entry *entry;
> > bool restart;
> >
> > if (!smp2p->ssr_ack_enabled)
> > return false;
> >
> > restart = in->flags & BIT(SMP2P_FLAGS_RESTART_DONE_BIT);
> > + restart = restart != smp2p->ssr_ack;
> > + if (restart && in->version > SMP2P_VERSION_1) {
> > + list_for_each_entry(entry, &smp2p->inbound, node) {
> > + if (!entry->value)
> > + continue;
> > + entry->last_value = 0;
> > + }
> > + }
> >
> > - return restart != smp2p->ssr_ack;
> > + return restart;
> > }
> >
> > static void qcom_smp2p_do_ssr_ack(struct qcom_smp2p *smp2p)
> > @@ -222,6 +234,20 @@ static void qcom_smp2p_negotiate(struct qcom_smp2p *smp2p)
> > }
> > }
> >
> > +static int qcom_smp2p_in_version(struct qcom_smp2p *smp2p)
> > +{
> > + unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
> > + unsigned int pid = smp2p->remote_pid;
> > + struct smp2p_smem_item *in;
> > + size_t size;
> > +
> > + in = qcom_smem_get(pid, smem_id, &size);
> > + if (IS_ERR(in))
> > + return 0;
> > +
> > + return in->version;
> > +}
> > +
> > static void qcom_smp2p_start_in(struct qcom_smp2p *smp2p)
> > {
> > unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
> > @@ -516,6 +542,7 @@ static int qcom_smp2p_alloc_outbound_item(struct qcom_smp2p *smp2p)
> > struct smp2p_smem_item *out;
> > unsigned smem_id = smp2p->smem_items[SMP2P_OUTBOUND];
> > unsigned pid = smp2p->remote_pid;
> > + u8 in_version;
> > int ret;
> >
> > ret = qcom_smem_alloc(pid, smem_id, sizeof(*out));
> > @@ -537,12 +564,14 @@ static int qcom_smp2p_alloc_outbound_item(struct qcom_smp2p *smp2p)
> > out->valid_entries = 0;
> > out->features = SMP2P_ALL_FEATURES;
> >
> > + in_version = qcom_smp2p_in_version(smp2p);
> > +
> > /*
> > * Make sure the rest of the header is written before we validate the
> > * item by writing a valid version number.
> > */
> > wmb();
> > - out->version = 1;
> > + out->version = (in_version) ? in_version : 1;
>
> Doesn't this imply that if the remoteproc advertises support for version
> 3, then we suddenly also support version 3?
>
Yea I think this is a result of a quick fix and certainty that no
firmware was running around with version 3. We can clean this up.
>
> I don't remember if we've talked about how version handling should work
> in this driver, but we should certainly define and document that in the
> comment at the top of this driver.
>
We did, there's provisions for the remote to version down or place a
0xFF value if it isn't capable of versioning down. Unfortunately I
think this versioning down behavior comes as part of V2.
Because we have to support older firmware, it was better to mirror V1
for older remotes in case they couldn't understand Linux doing the
version down sequence.
We don't have any remote with V3 so I didnt implement that part of the
V2 smp2p spec.
> Regards,
> Bjorn
>
> >
> > qcom_smp2p_kick(smp2p);
> >
> >
> > --
> > 2.25.1
> >
Powered by blists - more mailing lists