linux-kernel - Re: [RFC PATCH 1/3] usb: pd: Increase max PDO objects to 13

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]

Message-ID: <20210903064701.GA3515@jackp-linux.qualcomm.com>
Date:   Thu, 2 Sep 2021 23:47:01 -0700
From:   Jack Pham <jackp@...eaurora.org>
To:     Prashant Malani <pmalani@...omium.org>
Cc:     linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        linux-pm@...r.kernel.org, bleung@...omium.org,
        heikki.krogerus@...ux.intel.com, badhri@...gle.com,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Sebastian Reichel <sre@...nel.org>
Subject: Re: [RFC PATCH 1/3] usb: pd: Increase max PDO objects to 13

Hi Prashant,

On Thu, Sep 02, 2021 at 02:34:58PM -0700, Prashant Malani wrote:
> Increase the max number of PDO objects to 13, to accommodate the extra
> PDOs added as a part of EPR (Extended Power Range) operation introduced
> in the USB PD Spec Rev 3.1, v 1.0. See Figure 6-54 for details.
> 
> Signed-off-by: Prashant Malani <pmalani@...omium.org>
> ---
>  include/linux/usb/pd.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> index 96b7ff66f074..7e8bdca1ce6e 100644
> --- a/include/linux/usb/pd.h
> +++ b/include/linux/usb/pd.h
> @@ -201,7 +201,13 @@ struct pd_message {
>  } __packed;
>  
>  /* PDO: Power Data Object */
> -#define PDO_MAX_OBJECTS		7
> +
> +/*
> + * The EPR (Extended Power Range) structure is a superset of the SPR (Standard Power Range)
> + * capabilities structure, so set the max number of PDOs to 13 instead of 7. On SPR-only systems,
> + * objects 8 through 13 will just be empty.
> + */
> +#define PDO_MAX_OBJECTS		13

Hmm this might break the recent change I made to UCSI in commit
1f4642b72be7 ("usb: typec: ucsi: Retrieve all the PDOs instead of just
the first 4").

 520 static void ucsi_get_src_pdos(struct ucsi_connector *con, int is_partner)
 521 {
 522         int ret;
 523
 524         /* UCSI max payload means only getting at most 4 PDOs at a time */
 525         ret = ucsi_get_pdos(con, 1, con->src_pdos, 0, UCSI_MAX_PDOS);
 526         if (ret < 0)
 527                 return;
 528
 529         con->num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
 530         if (con->num_pdos < UCSI_MAX_PDOS)
 531                 return;
 532
 533         /* get the remaining PDOs, if any */
 534         ret = ucsi_get_pdos(con, 1, con->src_pdos, UCSI_MAX_PDOS,
 535                             PDO_MAX_OBJECTS - UCSI_MAX_PDOS);
				 ^^^^^^^^^^^^^^^
This routine calls the UCSI GET_PDOS command for up to 4 PDOs at a time
since that's the most the return payload can carry.  Currently this
assumes that we'd only need to request the PPM at most twice to retrieve
all the PDOs for up to a maximum of 7 (first request for 4 then again if
needed for the remaining 3).  I'm not sure if any existing UCSI FW would
be updatable to support more than 7 PDOs in the future, much less
support EPR.  In fact, current UCSI 1.2 spec [1] Table 4-34 mentions PDO
offset valid values are 0-7 and anything else "shall not be used", so I
don't know how UCSI will eventually cope with EPR without a spec update.

So if this macro changes to 13 then this call would result in a call to
the UCSI GET_PDOS command passing num_pdos == 13-4 = 9 which would
probably result in an error from the PPM FW.  So we might need to retain
the maximum value of 7 PDOs at least for UCSI here.  Maybe that means
this UCSI driver needs to carry its own definition of
UCSI_MAX_TOTAL_PDOS=7 instead of using PDO_MAX_OBJECTS?

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project