diff mbox series

[1/6] dt-bindings: remoteproc: qcom,pas-common: Introduce iommus and qcom,devmem property

Message ID 20241004212359.2263502-2-quic_mojha@quicinc.com (mailing list archive)
State Not Applicable
Headers show
Series Peripheral Image Loader support for Qualcomm SoCs | expand

Commit Message

Mukesh Ojha Oct. 4, 2024, 9:23 p.m. UTC
From: Shiraz Hashim <quic_shashim@quicinc.com>

Qualcomm’s PAS implementation for remote processors only supports a
single stage of IOMMU translation and is presently managed by the
Qualcomm EL2 hypervisor (QHEE) if it is present. In the absence of QHEE,
such as with a KVM hypervisor, IOMMU translations need to be set up by
the KVM host. Remoteproc needs carveout memory region and its resource
(device memory) permissions to be set before it comes up, and this
information is presently available statically with QHEE.

In the absence of QHEE, the boot firmware needs to overlay this
information based on SoCs running with either QHEE or a KVM hypervisor
(CPUs booted in EL2).

The qcom,devmem property provides IOMMU devmem translation information
intended for non-QHEE based systems.

Signed-off-by: Shiraz Hashim <quic_shashim@quicinc.com>
Co-Developed-by: Mukesh Ojha <quic_mojha@quicinc.com>
Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
---
 .../bindings/remoteproc/qcom,pas-common.yaml  | 42 +++++++++++++++++++
 .../bindings/remoteproc/qcom,sa8775p-pas.yaml | 20 +++++++++
 2 files changed, 62 insertions(+)

Comments

Dmitry Baryshkov Oct. 6, 2024, 7:38 p.m. UTC | #1
On Sat, Oct 05, 2024 at 02:53:54AM GMT, Mukesh Ojha wrote:
> From: Shiraz Hashim <quic_shashim@quicinc.com>
> 
> Qualcomm’s PAS implementation for remote processors only supports a
> single stage of IOMMU translation and is presently managed by the
> Qualcomm EL2 hypervisor (QHEE) if it is present. In the absence of QHEE,
> such as with a KVM hypervisor, IOMMU translations need to be set up by
> the KVM host. Remoteproc needs carveout memory region and its resource
> (device memory) permissions to be set before it comes up, and this
> information is presently available statically with QHEE.
> 
> In the absence of QHEE, the boot firmware needs to overlay this
> information based on SoCs running with either QHEE or a KVM hypervisor
> (CPUs booted in EL2).
> 
> The qcom,devmem property provides IOMMU devmem translation information
> intended for non-QHEE based systems.
> 
> Signed-off-by: Shiraz Hashim <quic_shashim@quicinc.com>
> Co-Developed-by: Mukesh Ojha <quic_mojha@quicinc.com>
> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> ---
>  .../bindings/remoteproc/qcom,pas-common.yaml  | 42 +++++++++++++++++++
>  .../bindings/remoteproc/qcom,sa8775p-pas.yaml | 20 +++++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> index 63a82e7a8bf8..068e177ad934 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> @@ -52,6 +52,48 @@ properties:
>      minItems: 1
>      maxItems: 3
>  
> +  iommus:
> +    maxItems: 1
> +
> +  qcom,devmem:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    description:
> +      Qualcomm’s PAS implementation for remote processors only supports a
> +      single stage of IOMMU translation and is presently managed by the
> +      Qualcomm EL2 hypervisor (QHEE) if it is present. In the absence of QHEE,
> +      such as with a KVM hypervisor, IOMMU translations need to be set up by
> +      the KVM host. Remoteproc might need some device resources and related
> +      access permissions to be set before it comes up, and this information is
> +      presently available statically with QHEE.
> +
> +      In the absence of QHEE, the boot firmware needs to overlay this
> +      information based on SoCs running with either QHEE or a KVM hypervisor
> +      (CPUs booted in EL2).
> +
> +      The qcom,devmem property provides IOMMU devmem translation information
> +      intended for non-QHEE based systems. It is an array of u32 values
> +      describing the device memory regions for which IOMMU translations need to
> +      be set up before bringing up Remoteproc. This array consists of 4-tuples
> +      defining the device address, physical address, size, and attribute flags
> +      with which it has to be mapped.

I'd expect that this kind of information is hardware-dependent. As such
it can go to the driver itself, rather than the device tree. The driver
can use compatible string to select the correct table.

> +
> +      remoteproc@3000000 {
> +          ...
> +
> +          qcom,devmem = <0x82000 0x82000 0x2000 0x3>,
> +                        <0x92000 0x92000 0x1000 0x1>;
> +      }
> +
> +    items:
> +      items:
> +        - description: device address
> +        - description: physical address
> +        - description: size of mapping
> +        - description: |
> +            iommu attributes - IOMMU_READ, IOMMU_WRITE, IOMMU_CACHE, IOMMU_NOEXEC, IOMMU_MMIO
> +          enum: [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
> +                  25, 26, 27, 28, 29, 30, 31 ]

Attributes should definitely be defined and then the DT should use
defines rather than the raw values.

> +
>    qcom,smem-states:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
>      description: States used by the AP to signal the Hexagon core
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sa8775p-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sa8775p-pas.yaml
> index 7fe401a06805..503c5c9d8ea7 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,sa8775p-pas.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,sa8775p-pas.yaml
> @@ -139,6 +139,26 @@ examples:
>          power-domains = <&rpmhpd RPMHPD_LCX>, <&rpmhpd RPMHPD_LMX>;
>          power-domain-names = "lcx", "lmx";
>  
> +        iommus = <&apps_smmu 0x3000 0x0>;
> +        qcom,devmem = <0x00110000 0x00110000 0x4000 0x1>,
> +                      <0x00123000 0x00123000 0x1000 0x3>,
> +                      <0x00124000 0x00124000 0x3000 0x3>,
> +                      <0x00127000 0x00127000 0x2000 0x3>,
> +                      <0x0012a000 0x0012a000 0x3000 0x3>,
> +                      <0x0012e000 0x0012e000 0x1000 0x3>,
> +                      <0x0012f000 0x0012f000 0x1000 0x1>,
> +                      <0x00144000 0x00144000 0x1000 0x1>,
> +                      <0x00148000 0x00148000 0x1000 0x1>,
> +                      <0x00149000 0x00149000 0xe000 0x3>,
> +                      <0x00157000 0x00157000 0x1000 0x3>,
> +                      <0x00158000 0x00158000 0xd000 0x3>,
> +                      <0x00165000 0x00165000 0x1000 0x3>,
> +                      <0x00172000 0x00172000 0x1000 0x3>,
> +                      <0x00173000 0x00173000 0x8000 0x3>,
> +                      <0x0017b000 0x0017b000 0x2000 0x3>,
> +                      <0x0017f000 0x0017f000 0x1000 0x3>,
> +                      <0x00184000 0x00184000 0x1000 0x1>;
> +
>          interconnects = <&lpass_ag_noc MASTER_LPASS_PROC 0 &mc_virt SLAVE_EBI1 0>;
>  
>          memory-region = <&pil_adsp_mem>;
> -- 
> 2.34.1
>
Mukesh Ojha Oct. 7, 2024, 3:35 p.m. UTC | #2
On Sun, Oct 06, 2024 at 10:38:01PM +0300, Dmitry Baryshkov wrote:
> On Sat, Oct 05, 2024 at 02:53:54AM GMT, Mukesh Ojha wrote:
> > From: Shiraz Hashim <quic_shashim@quicinc.com>
> > 
> > Qualcomm’s PAS implementation for remote processors only supports a
> > single stage of IOMMU translation and is presently managed by the
> > Qualcomm EL2 hypervisor (QHEE) if it is present. In the absence of QHEE,
> > such as with a KVM hypervisor, IOMMU translations need to be set up by
> > the KVM host. Remoteproc needs carveout memory region and its resource
> > (device memory) permissions to be set before it comes up, and this
> > information is presently available statically with QHEE.
> > 
> > In the absence of QHEE, the boot firmware needs to overlay this
> > information based on SoCs running with either QHEE or a KVM hypervisor
> > (CPUs booted in EL2).
> > 
> > The qcom,devmem property provides IOMMU devmem translation information
> > intended for non-QHEE based systems.
> > 
> > Signed-off-by: Shiraz Hashim <quic_shashim@quicinc.com>
> > Co-Developed-by: Mukesh Ojha <quic_mojha@quicinc.com>
> > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> > ---
> >  .../bindings/remoteproc/qcom,pas-common.yaml  | 42 +++++++++++++++++++
> >  .../bindings/remoteproc/qcom,sa8775p-pas.yaml | 20 +++++++++
> >  2 files changed, 62 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> > index 63a82e7a8bf8..068e177ad934 100644
> > --- a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> > @@ -52,6 +52,48 @@ properties:
> >      minItems: 1
> >      maxItems: 3
> >  
> > +  iommus:
> > +    maxItems: 1
> > +
> > +  qcom,devmem:
> > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +    description:
> > +      Qualcomm’s PAS implementation for remote processors only supports a
> > +      single stage of IOMMU translation and is presently managed by the
> > +      Qualcomm EL2 hypervisor (QHEE) if it is present. In the absence of QHEE,
> > +      such as with a KVM hypervisor, IOMMU translations need to be set up by
> > +      the KVM host. Remoteproc might need some device resources and related
> > +      access permissions to be set before it comes up, and this information is
> > +      presently available statically with QHEE.
> > +
> > +      In the absence of QHEE, the boot firmware needs to overlay this
> > +      information based on SoCs running with either QHEE or a KVM hypervisor
> > +      (CPUs booted in EL2).
> > +
> > +      The qcom,devmem property provides IOMMU devmem translation information
> > +      intended for non-QHEE based systems. It is an array of u32 values
> > +      describing the device memory regions for which IOMMU translations need to
> > +      be set up before bringing up Remoteproc. This array consists of 4-tuples
> > +      defining the device address, physical address, size, and attribute flags
> > +      with which it has to be mapped.
> 
> I'd expect that this kind of information is hardware-dependent. As such
> it can go to the driver itself, rather than the device tree. The driver
> can use compatible string to select the correct table.
> 

IIUC, are you saying that to move this into driver file and override the
compatible string via overlay ?

> > +
> > +      remoteproc@3000000 {
> > +          ...
> > +
> > +          qcom,devmem = <0x82000 0x82000 0x2000 0x3>,
> > +                        <0x92000 0x92000 0x1000 0x1>;
> > +      }
> > +
> > +    items:
> > +      items:
> > +        - description: device address
> > +        - description: physical address
> > +        - description: size of mapping
> > +        - description: |
> > +            iommu attributes - IOMMU_READ, IOMMU_WRITE, IOMMU_CACHE, IOMMU_NOEXEC, IOMMU_MMIO
> > +          enum: [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
> > +                  25, 26, 27, 28, 29, 30, 31 ]
> 
> Attributes should definitely be defined and then the DT should use
> defines rather than the raw values.
>

Ack.

-Mukesh
Dmitry Baryshkov Oct. 7, 2024, 4:25 p.m. UTC | #3
On Mon, 7 Oct 2024 at 17:35, Mukesh Ojha <quic_mojha@quicinc.com> wrote:
>
> On Sun, Oct 06, 2024 at 10:38:01PM +0300, Dmitry Baryshkov wrote:
> > On Sat, Oct 05, 2024 at 02:53:54AM GMT, Mukesh Ojha wrote:
> > > From: Shiraz Hashim <quic_shashim@quicinc.com>
> > >
> > > Qualcomm’s PAS implementation for remote processors only supports a
> > > single stage of IOMMU translation and is presently managed by the
> > > Qualcomm EL2 hypervisor (QHEE) if it is present. In the absence of QHEE,
> > > such as with a KVM hypervisor, IOMMU translations need to be set up by
> > > the KVM host. Remoteproc needs carveout memory region and its resource
> > > (device memory) permissions to be set before it comes up, and this
> > > information is presently available statically with QHEE.
> > >
> > > In the absence of QHEE, the boot firmware needs to overlay this
> > > information based on SoCs running with either QHEE or a KVM hypervisor
> > > (CPUs booted in EL2).
> > >
> > > The qcom,devmem property provides IOMMU devmem translation information
> > > intended for non-QHEE based systems.
> > >
> > > Signed-off-by: Shiraz Hashim <quic_shashim@quicinc.com>
> > > Co-Developed-by: Mukesh Ojha <quic_mojha@quicinc.com>
> > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> > > ---
> > >  .../bindings/remoteproc/qcom,pas-common.yaml  | 42 +++++++++++++++++++
> > >  .../bindings/remoteproc/qcom,sa8775p-pas.yaml | 20 +++++++++
> > >  2 files changed, 62 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> > > index 63a82e7a8bf8..068e177ad934 100644
> > > --- a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> > > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> > > @@ -52,6 +52,48 @@ properties:
> > >      minItems: 1
> > >      maxItems: 3
> > >
> > > +  iommus:
> > > +    maxItems: 1
> > > +
> > > +  qcom,devmem:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > +    description:
> > > +      Qualcomm’s PAS implementation for remote processors only supports a
> > > +      single stage of IOMMU translation and is presently managed by the
> > > +      Qualcomm EL2 hypervisor (QHEE) if it is present. In the absence of QHEE,
> > > +      such as with a KVM hypervisor, IOMMU translations need to be set up by
> > > +      the KVM host. Remoteproc might need some device resources and related
> > > +      access permissions to be set before it comes up, and this information is
> > > +      presently available statically with QHEE.
> > > +
> > > +      In the absence of QHEE, the boot firmware needs to overlay this
> > > +      information based on SoCs running with either QHEE or a KVM hypervisor
> > > +      (CPUs booted in EL2).
> > > +
> > > +      The qcom,devmem property provides IOMMU devmem translation information
> > > +      intended for non-QHEE based systems. It is an array of u32 values
> > > +      describing the device memory regions for which IOMMU translations need to
> > > +      be set up before bringing up Remoteproc. This array consists of 4-tuples
> > > +      defining the device address, physical address, size, and attribute flags
> > > +      with which it has to be mapped.
> >
> > I'd expect that this kind of information is hardware-dependent. As such
> > it can go to the driver itself, rather than the device tree. The driver
> > can use compatible string to select the correct table.
> >
>
> IIUC, are you saying that to move this into driver file and override the
> compatible string via overlay ?

Ideally we should live without compat overrides. On the other hand,
sc7180 and sc7280 provide an example of doing exactly that.

>
> > > +
> > > +      remoteproc@3000000 {
> > > +          ...
> > > +
> > > +          qcom,devmem = <0x82000 0x82000 0x2000 0x3>,
> > > +                        <0x92000 0x92000 0x1000 0x1>;
> > > +      }
> > > +
> > > +    items:
> > > +      items:
> > > +        - description: device address
> > > +        - description: physical address
> > > +        - description: size of mapping
> > > +        - description: |
> > > +            iommu attributes - IOMMU_READ, IOMMU_WRITE, IOMMU_CACHE, IOMMU_NOEXEC, IOMMU_MMIO
> > > +          enum: [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
> > > +                  25, 26, 27, 28, 29, 30, 31 ]
> >
> > Attributes should definitely be defined and then the DT should use
> > defines rather than the raw values.
> >
>
> Ack.
>
> -Mukesh
>
Shiraz Hashim Oct. 9, 2024, 2:04 p.m. UTC | #4
On Mon, Oct 07, 2024 at 06:25:01PM +0200, Dmitry Baryshkov wrote:
> On Mon, 7 Oct 2024 at 17:35, Mukesh Ojha <quic_mojha@quicinc.com> wrote:
> >
> > On Sun, Oct 06, 2024 at 10:38:01PM +0300, Dmitry Baryshkov wrote:
> > > On Sat, Oct 05, 2024 at 02:53:54AM GMT, Mukesh Ojha wrote:
> > > > From: Shiraz Hashim <quic_shashim@quicinc.com>
> > > >
> > > > Qualcomm’s PAS implementation for remote processors only supports a
> > > > single stage of IOMMU translation and is presently managed by the
> > > > Qualcomm EL2 hypervisor (QHEE) if it is present. In the absence of QHEE,
> > > > such as with a KVM hypervisor, IOMMU translations need to be set up by
> > > > the KVM host. Remoteproc needs carveout memory region and its resource
> > > > (device memory) permissions to be set before it comes up, and this
> > > > information is presently available statically with QHEE.
> > > >
> > > > In the absence of QHEE, the boot firmware needs to overlay this
> > > > information based on SoCs running with either QHEE or a KVM hypervisor
> > > > (CPUs booted in EL2).
> > > >
> > > > The qcom,devmem property provides IOMMU devmem translation information
> > > > intended for non-QHEE based systems.
> > > >
> > > > Signed-off-by: Shiraz Hashim <quic_shashim@quicinc.com>
> > > > Co-Developed-by: Mukesh Ojha <quic_mojha@quicinc.com>
> > > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> > > > ---
> > > >  .../bindings/remoteproc/qcom,pas-common.yaml  | 42 +++++++++++++++++++
> > > >  .../bindings/remoteproc/qcom,sa8775p-pas.yaml | 20 +++++++++
> > > >  2 files changed, 62 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> > > > index 63a82e7a8bf8..068e177ad934 100644
> > > > --- a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> > > > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> > > > @@ -52,6 +52,48 @@ properties:
> > > >      minItems: 1
> > > >      maxItems: 3
> > > >
> > > > +  iommus:
> > > > +    maxItems: 1
> > > > +
> > > > +  qcom,devmem:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > > +    description:
> > > > +      Qualcomm’s PAS implementation for remote processors only supports a
> > > > +      single stage of IOMMU translation and is presently managed by the
> > > > +      Qualcomm EL2 hypervisor (QHEE) if it is present. In the absence of QHEE,
> > > > +      such as with a KVM hypervisor, IOMMU translations need to be set up by
> > > > +      the KVM host. Remoteproc might need some device resources and related
> > > > +      access permissions to be set before it comes up, and this information is
> > > > +      presently available statically with QHEE.
> > > > +
> > > > +      In the absence of QHEE, the boot firmware needs to overlay this
> > > > +      information based on SoCs running with either QHEE or a KVM hypervisor
> > > > +      (CPUs booted in EL2).
> > > > +
> > > > +      The qcom,devmem property provides IOMMU devmem translation information
> > > > +      intended for non-QHEE based systems. It is an array of u32 values
> > > > +      describing the device memory regions for which IOMMU translations need to
> > > > +      be set up before bringing up Remoteproc. This array consists of 4-tuples
> > > > +      defining the device address, physical address, size, and attribute flags
> > > > +      with which it has to be mapped.
> > >
> > > I'd expect that this kind of information is hardware-dependent. As such
> > > it can go to the driver itself, rather than the device tree. The driver
> > > can use compatible string to select the correct table.
> > >
> >
> > IIUC, are you saying that to move this into driver file and override the
> > compatible string via overlay ?
> 
> Ideally we should live without compat overrides. On the other hand,
> sc7180 and sc7280 provide an example of doing exactly that.

I am not sure if there can arise a case where updated adsp firmware
for particular board(s) may require additional access.

Having it in device tree adds a convenience to deal with such
variance. 

regards
Shiraz
Krzysztof Kozlowski Oct. 10, 2024, 7:15 a.m. UTC | #5
On 09/10/2024 16:04, Shiraz Hashim wrote:
> On Mon, Oct 07, 2024 at 06:25:01PM +0200, Dmitry Baryshkov wrote:
>> On Mon, 7 Oct 2024 at 17:35, Mukesh Ojha <quic_mojha@quicinc.com> wrote:
>>>
>>> On Sun, Oct 06, 2024 at 10:38:01PM +0300, Dmitry Baryshkov wrote:
>>>> On Sat, Oct 05, 2024 at 02:53:54AM GMT, Mukesh Ojha wrote:
>>>>> From: Shiraz Hashim <quic_shashim@quicinc.com>
>>>>>
>>>>> Qualcomm’s PAS implementation for remote processors only supports a
>>>>> single stage of IOMMU translation and is presently managed by the
>>>>> Qualcomm EL2 hypervisor (QHEE) if it is present. In the absence of QHEE,
>>>>> such as with a KVM hypervisor, IOMMU translations need to be set up by
>>>>> the KVM host. Remoteproc needs carveout memory region and its resource
>>>>> (device memory) permissions to be set before it comes up, and this
>>>>> information is presently available statically with QHEE.
>>>>>
>>>>> In the absence of QHEE, the boot firmware needs to overlay this
>>>>> information based on SoCs running with either QHEE or a KVM hypervisor
>>>>> (CPUs booted in EL2).
>>>>>
>>>>> The qcom,devmem property provides IOMMU devmem translation information
>>>>> intended for non-QHEE based systems.
>>>>>
>>>>> Signed-off-by: Shiraz Hashim <quic_shashim@quicinc.com>
>>>>> Co-Developed-by: Mukesh Ojha <quic_mojha@quicinc.com>
>>>>> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
>>>>> ---
>>>>>  .../bindings/remoteproc/qcom,pas-common.yaml  | 42 +++++++++++++++++++
>>>>>  .../bindings/remoteproc/qcom,sa8775p-pas.yaml | 20 +++++++++
>>>>>  2 files changed, 62 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
>>>>> index 63a82e7a8bf8..068e177ad934 100644
>>>>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
>>>>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
>>>>> @@ -52,6 +52,48 @@ properties:
>>>>>      minItems: 1
>>>>>      maxItems: 3
>>>>>
>>>>> +  iommus:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  qcom,devmem:
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
>>>>> +    description:
>>>>> +      Qualcomm’s PAS implementation for remote processors only supports a
>>>>> +      single stage of IOMMU translation and is presently managed by the
>>>>> +      Qualcomm EL2 hypervisor (QHEE) if it is present. In the absence of QHEE,
>>>>> +      such as with a KVM hypervisor, IOMMU translations need to be set up by
>>>>> +      the KVM host. Remoteproc might need some device resources and related
>>>>> +      access permissions to be set before it comes up, and this information is
>>>>> +      presently available statically with QHEE.
>>>>> +
>>>>> +      In the absence of QHEE, the boot firmware needs to overlay this
>>>>> +      information based on SoCs running with either QHEE or a KVM hypervisor
>>>>> +      (CPUs booted in EL2).
>>>>> +
>>>>> +      The qcom,devmem property provides IOMMU devmem translation information
>>>>> +      intended for non-QHEE based systems. It is an array of u32 values
>>>>> +      describing the device memory regions for which IOMMU translations need to
>>>>> +      be set up before bringing up Remoteproc. This array consists of 4-tuples
>>>>> +      defining the device address, physical address, size, and attribute flags
>>>>> +      with which it has to be mapped.
>>>>
>>>> I'd expect that this kind of information is hardware-dependent. As such
>>>> it can go to the driver itself, rather than the device tree. The driver
>>>> can use compatible string to select the correct table.
>>>>
>>>
>>> IIUC, are you saying that to move this into driver file and override the
>>> compatible string via overlay ?
>>
>> Ideally we should live without compat overrides. On the other hand,
>> sc7180 and sc7280 provide an example of doing exactly that.
> 
> I am not sure if there can arise a case where updated adsp firmware
> for particular board(s) may require additional access.
> 
> Having it in device tree adds a convenience to deal with such
> variance. 
> 

That's a downstream argument... Just look at the downstream DTS.
Everything, even software properties, can be added to DT, right?

Best regards,
Krzysztof
Shiraz Hashim Oct. 10, 2024, 8:30 a.m. UTC | #6
On Thu, Oct 10, 2024 at 09:15:59AM +0200, Krzysztof Kozlowski wrote:
> On 09/10/2024 16:04, Shiraz Hashim wrote:
> > On Mon, Oct 07, 2024 at 06:25:01PM +0200, Dmitry Baryshkov wrote:
> >> On Mon, 7 Oct 2024 at 17:35, Mukesh Ojha <quic_mojha@quicinc.com> wrote:
> >>>
> >>> On Sun, Oct 06, 2024 at 10:38:01PM +0300, Dmitry Baryshkov wrote:
> >>>> On Sat, Oct 05, 2024 at 02:53:54AM GMT, Mukesh Ojha wrote:
> >>>>> From: Shiraz Hashim <quic_shashim@quicinc.com>
> >>>>>
> >>>>> Qualcomm’s PAS implementation for remote processors only supports a
> >>>>> single stage of IOMMU translation and is presently managed by the
> >>>>> Qualcomm EL2 hypervisor (QHEE) if it is present. In the absence of QHEE,
> >>>>> such as with a KVM hypervisor, IOMMU translations need to be set up by
> >>>>> the KVM host. Remoteproc needs carveout memory region and its resource
> >>>>> (device memory) permissions to be set before it comes up, and this
> >>>>> information is presently available statically with QHEE.
> >>>>>
> >>>>> In the absence of QHEE, the boot firmware needs to overlay this
> >>>>> information based on SoCs running with either QHEE or a KVM hypervisor
> >>>>> (CPUs booted in EL2).
> >>>>>
> >>>>> The qcom,devmem property provides IOMMU devmem translation information
> >>>>> intended for non-QHEE based systems.
> >>>>>
> >>>>> Signed-off-by: Shiraz Hashim <quic_shashim@quicinc.com>
> >>>>> Co-Developed-by: Mukesh Ojha <quic_mojha@quicinc.com>
> >>>>> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> >>>>> ---
> >>>>>  .../bindings/remoteproc/qcom,pas-common.yaml  | 42 +++++++++++++++++++
> >>>>>  .../bindings/remoteproc/qcom,sa8775p-pas.yaml | 20 +++++++++
> >>>>>  2 files changed, 62 insertions(+)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> >>>>> index 63a82e7a8bf8..068e177ad934 100644
> >>>>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> >>>>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
> >>>>> @@ -52,6 +52,48 @@ properties:
> >>>>>      minItems: 1
> >>>>>      maxItems: 3
> >>>>>
> >>>>> +  iommus:
> >>>>> +    maxItems: 1
> >>>>> +
> >>>>> +  qcom,devmem:
> >>>>> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> >>>>> +    description:
> >>>>> +      Qualcomm’s PAS implementation for remote processors only supports a
> >>>>> +      single stage of IOMMU translation and is presently managed by the
> >>>>> +      Qualcomm EL2 hypervisor (QHEE) if it is present. In the absence of QHEE,
> >>>>> +      such as with a KVM hypervisor, IOMMU translations need to be set up by
> >>>>> +      the KVM host. Remoteproc might need some device resources and related
> >>>>> +      access permissions to be set before it comes up, and this information is
> >>>>> +      presently available statically with QHEE.
> >>>>> +
> >>>>> +      In the absence of QHEE, the boot firmware needs to overlay this
> >>>>> +      information based on SoCs running with either QHEE or a KVM hypervisor
> >>>>> +      (CPUs booted in EL2).
> >>>>> +
> >>>>> +      The qcom,devmem property provides IOMMU devmem translation information
> >>>>> +      intended for non-QHEE based systems. It is an array of u32 values
> >>>>> +      describing the device memory regions for which IOMMU translations need to
> >>>>> +      be set up before bringing up Remoteproc. This array consists of 4-tuples
> >>>>> +      defining the device address, physical address, size, and attribute flags
> >>>>> +      with which it has to be mapped.
> >>>>
> >>>> I'd expect that this kind of information is hardware-dependent. As such
> >>>> it can go to the driver itself, rather than the device tree. The driver
> >>>> can use compatible string to select the correct table.
> >>>>
> >>>
> >>> IIUC, are you saying that to move this into driver file and override the
> >>> compatible string via overlay ?
> >>
> >> Ideally we should live without compat overrides. On the other hand,
> >> sc7180 and sc7280 provide an example of doing exactly that.
> > 
> > I am not sure if there can arise a case where updated adsp firmware
> > for particular board(s) may require additional access.
> > 
> > Having it in device tree adds a convenience to deal with such
> > variance. 
> > 
> 
> That's a downstream argument... Just look at the downstream DTS.
> Everything, even software properties, can be added to DT, right?

I was thinking this binding is similar to iommu-addresses approach,
however that is under reserved-memory enumerating memory regions.

regards
Shiraz
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
index 63a82e7a8bf8..068e177ad934 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
@@ -52,6 +52,48 @@  properties:
     minItems: 1
     maxItems: 3
 
+  iommus:
+    maxItems: 1
+
+  qcom,devmem:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    description:
+      Qualcomm’s PAS implementation for remote processors only supports a
+      single stage of IOMMU translation and is presently managed by the
+      Qualcomm EL2 hypervisor (QHEE) if it is present. In the absence of QHEE,
+      such as with a KVM hypervisor, IOMMU translations need to be set up by
+      the KVM host. Remoteproc might need some device resources and related
+      access permissions to be set before it comes up, and this information is
+      presently available statically with QHEE.
+
+      In the absence of QHEE, the boot firmware needs to overlay this
+      information based on SoCs running with either QHEE or a KVM hypervisor
+      (CPUs booted in EL2).
+
+      The qcom,devmem property provides IOMMU devmem translation information
+      intended for non-QHEE based systems. It is an array of u32 values
+      describing the device memory regions for which IOMMU translations need to
+      be set up before bringing up Remoteproc. This array consists of 4-tuples
+      defining the device address, physical address, size, and attribute flags
+      with which it has to be mapped.
+
+      remoteproc@3000000 {
+          ...
+
+          qcom,devmem = <0x82000 0x82000 0x2000 0x3>,
+                        <0x92000 0x92000 0x1000 0x1>;
+      }
+
+    items:
+      items:
+        - description: device address
+        - description: physical address
+        - description: size of mapping
+        - description: |
+            iommu attributes - IOMMU_READ, IOMMU_WRITE, IOMMU_CACHE, IOMMU_NOEXEC, IOMMU_MMIO
+          enum: [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
+                  25, 26, 27, 28, 29, 30, 31 ]
+
   qcom,smem-states:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     description: States used by the AP to signal the Hexagon core
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sa8775p-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sa8775p-pas.yaml
index 7fe401a06805..503c5c9d8ea7 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,sa8775p-pas.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,sa8775p-pas.yaml
@@ -139,6 +139,26 @@  examples:
         power-domains = <&rpmhpd RPMHPD_LCX>, <&rpmhpd RPMHPD_LMX>;
         power-domain-names = "lcx", "lmx";
 
+        iommus = <&apps_smmu 0x3000 0x0>;
+        qcom,devmem = <0x00110000 0x00110000 0x4000 0x1>,
+                      <0x00123000 0x00123000 0x1000 0x3>,
+                      <0x00124000 0x00124000 0x3000 0x3>,
+                      <0x00127000 0x00127000 0x2000 0x3>,
+                      <0x0012a000 0x0012a000 0x3000 0x3>,
+                      <0x0012e000 0x0012e000 0x1000 0x3>,
+                      <0x0012f000 0x0012f000 0x1000 0x1>,
+                      <0x00144000 0x00144000 0x1000 0x1>,
+                      <0x00148000 0x00148000 0x1000 0x1>,
+                      <0x00149000 0x00149000 0xe000 0x3>,
+                      <0x00157000 0x00157000 0x1000 0x3>,
+                      <0x00158000 0x00158000 0xd000 0x3>,
+                      <0x00165000 0x00165000 0x1000 0x3>,
+                      <0x00172000 0x00172000 0x1000 0x3>,
+                      <0x00173000 0x00173000 0x8000 0x3>,
+                      <0x0017b000 0x0017b000 0x2000 0x3>,
+                      <0x0017f000 0x0017f000 0x1000 0x3>,
+                      <0x00184000 0x00184000 0x1000 0x1>;
+
         interconnects = <&lpass_ag_noc MASTER_LPASS_PROC 0 &mc_virt SLAVE_EBI1 0>;
 
         memory-region = <&pil_adsp_mem>;