From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.5 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E5E72C433B4 for ; Fri, 30 Apr 2021 21:01:33 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6633E613C8 for ; Fri, 30 Apr 2021 21:01:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6633E613C8 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=openbmc-bounces+openbmc=archiver.kernel.org@lists.ozlabs.org Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4FX4Y80gD6z3018 for ; Sat, 1 May 2021 07:01:32 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20161025 header.b=cySUWPUW; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=google.com (client-ip=2607:f8b0:4864:20::b2d; helo=mail-yb1-xb2d.google.com; envelope-from=jasonling@google.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20161025 header.b=cySUWPUW; dkim-atps=neutral Received: from mail-yb1-xb2d.google.com (mail-yb1-xb2d.google.com [IPv6:2607:f8b0:4864:20::b2d]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4FX4Xh4GxRz2y6N for ; Sat, 1 May 2021 07:01:06 +1000 (AEST) Received: by mail-yb1-xb2d.google.com with SMTP id x131so3788392ybg.11 for ; Fri, 30 Apr 2021 14:01:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=yEqSJnPqkZZ+epZbG76Y2KBE96Bliei3AXjc2aSEdPY=; b=cySUWPUWq+9MteqcnHpXSGqghzqhb+HcW2pwBNXlU/MynifsWSuSGPFkMh9lH9uWlv BGuGGbTCKgmldjsJ9y46CM+yWv4XqN8xdgc73Fy29fP0M3c5x3DwSqnSNyqDf2NSduXC JyRwIGCkL+L5evkweViIiBxTfbkKoLTRO8BzduxbeWmFs9y3bUAnJ0dMYv49bUCKrJjF dB767dKL4yHlQ1ZyPMs0mV2jQqwpsZbKMFygu1Zl+sBQv9NDCAOYXp5J3/LMnfcNmtxM cYN99fmOsFzWZL6/rBN9heBSjQDIcnYhD3/I/ElEbu3oBRhhYjNOFVU21CEA2xwRCzn9 gKWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=yEqSJnPqkZZ+epZbG76Y2KBE96Bliei3AXjc2aSEdPY=; b=mltp1KeSsOxCAL6AN3ScPhvvxsYxO5xosQe4Cj4xFog/2VJg4uzLAaG/CElxzdrPRV sr7bfomtP1Y4OC44b1NV4ztlyS/tP77baPp3zFl1x+sOmWRRW5w/Id0N7UPlj/OKJb64 0yO15heCMQUxSoxcdmCNjLPO/VUQUUeC9iuENDa6/onWBD3YbZS32wK6wpNiEZIiYIEG +r8pSuaeIpLERjju7AoCeL/EsfLsAk7sierzp97BRX3Q84I4oU/WNKP21dIiNXbcZfEM MByy7TqvF1qCaRiuRa/gOoiEG7wKR9dZH7n/876vHBUodApcOU3Vw66qbTFUmHyGfpwr IEwA== X-Gm-Message-State: AOAM533A4CNHv8dbinw+00NfY1Wee5t1wR+h4jlPgcTGZF9lM6V4Dvue TshHJ2KMo4TGk0eWbj5ADjuEsp8TQ5XlsMqyQzNmMg== X-Google-Smtp-Source: ABdhPJx+mvcfpGjF3nk8jlUPHee1g68vQiQ3ou1qQSO0vghk9KenvUlDddjQMPfWvKXVRACNpGUQodtPQ1k9e0MfBLc= X-Received: by 2002:a25:6084:: with SMTP id u126mr9523249ybb.198.1619816462516; Fri, 30 Apr 2021 14:01:02 -0700 (PDT) MIME-Version: 1.0 References: <2CAF9521-76FA-4160-9711-4267341B018A@gmail.com> In-Reply-To: From: Jason Ling Date: Fri, 30 Apr 2021 14:00:26 -0700 Message-ID: Subject: Re: [pmbusplus] userspace i2c, pmbus interactions To: Patrick Williams Content-Type: multipart/alternative; boundary="000000000000d9fc9f05c136e8d8" X-BeenThere: openbmc@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development list for OpenBMC List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: OpenBMC Maillist , Mike Jones Errors-To: openbmc-bounces+openbmc=archiver.kernel.org@lists.ozlabs.org Sender: "openbmc" --000000000000d9fc9f05c136e8d8 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable > > > However, in general, ADI PMBus devices have a PAGE command, especially > the LTC388X and LTC297X families. > > > > This means that many actions require locking the bus to create compound > transactions or the use of PAGE_PLUS. > > > > The In System Programming code we support on linux via /dev/i2c has thi= s > issue. If another process touched hwmon during programming, and touches a > PAGE, the programming may fail, or worse it sends the wrong settings. > I'm not sure how this is even possible. How did an hwmon driver touch > the device and userspace access it? I'm fairly sure. IIRC there is some protection if you use the smbus_readX smbus_writeX APIs but if you're doing ioctl(I2C_RDWR) then a hwmon driver being bound to a device won't stop you from hammering on it. Also userspace can pass the force option to i2c-dev and it'll let you hammer away on the device as well. On Fri, Apr 30, 2021 at 1:54 PM Patrick Williams wrote: > On Fri, Apr 30, 2021 at 09:43:21AM -0600, Mike Jones wrote: > > > I had a similar discussion with Guenter, who suggested that any i2c cod= e > in user space is problematic, but I think it also said there was enough > locking in the drivers that SMBus transactions from /dev/i2c or i2c-tools > would not interact with hwmon. > > I'm pretty sure there is locking such that the kernel won't let you talk > to a > device that is already bound to a driver. Once you unbind the kernel sid= e > you're free to do what you want. > > What I don't recall is how much of the i2c-mux support gets propagated > out to userspace. Hopefully you don't have to mess with moving muxes > and talking past those devices. (I've seen some nasty shell scripts > using i2c-tools doing things like this in the past.) > > > However, in general, ADI PMBus devices have a PAGE command, especially > the LTC388X and LTC297X families. > > > > This means that many actions require locking the bus to create compound > transactions or the use of PAGE_PLUS. > > > > The In System Programming code we support on linux via /dev/i2c has thi= s > issue. If another process touched hwmon during programming, and touches a > PAGE, the programming may fail, or worse it sends the wrong settings. > > I'm not sure how this is even possible. How did an hwmon driver touch > the device and userspace access it? I'm fairly sure. > > > If the work within the PMBus community to have a standard programming > file format, if it were to be applied in user space, OBMC would have to > disable hwmon and all telemetry while the programming is in process. > > > > If the file (or data blob) was applied in a kernel driver, it could loc= k > the i2c during the process so that all hwmon activity and telemetry are > held off. > > Right. I would expect the locking at a pmbus level so that the > pmbus-hwmon parts would block on a mutex until the firmware update is > complete (if firmware update were done in the pmbus driver). > > > This problem is not unique to our desire for a file format. That is > driven by the vendors complexity and business models. > > > > But, in the context of the ADM1266, it is a multipage device. The DS > does not say it can do PAGE_PLUS. Therefore, unless I am missing somethin= g, > the above problems apply unless using a PAGEless bulk programming > mechanism. I don=E2=80=99t know this part well enough to know how that wo= rks, or if > it is published. > > > > The other two families LTC388X and LTC297X do have a PAGEless bulk > programming. We don=E2=80=99t like do it with telemetry hammering it, mai= nly > because it feels risky. > > > > Finally, one could argue that most OMBC systems are using the 1266 and > not the other parts. But I can say I have sent patches for other parts to > OBMC users, so they are in use. > > There is already some firmware blob support in the kernel, just not for > pmbus. Has anyone investigated what (if anything) we'd need to do to be > able to leverage this? > > -- > Patrick Williams > --000000000000d9fc9f05c136e8d8 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
> However, in general, ADI= PMBus devices have a PAGE command, especially the LTC388X and LTC297X fami= lies.
><= br>> This m= eans that many actions require locking the bus to create compound transacti= ons or the use of PAGE_PLUS.
>
> The In System Programming code we support on linux via /de= v/i2c has this issue. If another process touched hwmon during programming, = and touches a PAGE, the programming may fail, or worse it sends the wrong s= ettings.
I'm not sure how this is even possible.=C2=A0 How did an hwmon dri= ver touch
the device and userspace access it?=C2=A0 I'm fairly sure.=

IIRC there is some protection if you use t= he smbus_readX smbus_writeX APIs but if you're doing ioctl(I2C_RDWR) th= en a hwmon driver being bound to a device won't stop you from hammering= on it.
Also userspace can pass the force option to i2c-dev and i= t'll let you hammer away on the device as well.=C2=A0

On Fri, Apr = 30, 2021 at 1:54 PM Patrick Williams <patrick@stwcx.xyz> wrote:
On Fri, Apr 30, 2021 at 09:43:21AM -0600, Mike Jones wr= ote:

> I had a similar discussion with Guenter, who suggested that any i2c co= de in user space is problematic, but I think it also said there was enough = locking in the drivers that SMBus transactions from /dev/i2c or i2c-tools w= ould not interact with hwmon.

I'm pretty sure there is locking such that the kernel won't let you= talk to a
device that is already bound to a driver.=C2=A0 Once you unbind the kernel = side
you're free to do what you want.

What I don't recall is how much of the i2c-mux support gets propagated<= br> out to userspace.=C2=A0 Hopefully you don't have to mess with moving mu= xes
and talking past those devices.=C2=A0 (I've seen some nasty shell scrip= ts
using i2c-tools doing things like this in the past.)

> However, in general, ADI PMBus devices have a PAGE command, especially= the LTC388X and LTC297X families.
>
> This means that many actions require locking the bus to create compoun= d transactions or the use of PAGE_PLUS.
>
> The In System Programming code we support on linux via /dev/i2c has th= is issue. If another process touched hwmon during programming, and touches = a PAGE, the programming may fail, or worse it sends the wrong settings.

I'm not sure how this is even possible.=C2=A0 How did an hwmon driver t= ouch
the device and userspace access it?=C2=A0 I'm fairly sure.

> If the work within the PMBus community to have a standard programming = file format, if it were to be applied in user space, OBMC would have to dis= able hwmon and all telemetry while the programming is in process.
>
> If the file (or data blob) was applied in a kernel driver, it could lo= ck the i2c during the process so that all hwmon activity and telemetry are = held off.

Right.=C2=A0 I would expect the locking at a pmbus level so that the
pmbus-hwmon parts would block on a mutex until the firmware update is
complete (if firmware update were done in the pmbus driver).

> This problem is not unique to our desire for a file format. That is dr= iven by the vendors complexity and business models.
>
> But, in the context of the ADM1266, it is a multipage device. The DS d= oes not say it can do PAGE_PLUS. Therefore, unless I am missing something, = the above problems apply unless using a PAGEless bulk programming mechanism= . I don=E2=80=99t know this part well enough to know how that works, or if = it is published.
>
> The other two families LTC388X and LTC297X do have a PAGEless bulk pro= gramming. We don=E2=80=99t like do it with telemetry hammering it, mainly b= ecause it feels risky.
>
> Finally, one could argue that most OMBC systems are using the 1266 and= not the other parts. But I can say I have sent patches for other parts to = OBMC users, so they are in use.

There is already some firmware blob support in the kernel, just not for
pmbus.=C2=A0 Has anyone investigated what (if anything) we'd need to do= to be
able to leverage this?

--
Patrick Williams
--000000000000d9fc9f05c136e8d8--