mbox series

[0/2] Add qcom hvc/shmem transport

Message ID 20230718160833.36397-1-quic_nkela@quicinc.com
Headers show
Series Add qcom hvc/shmem transport | expand

Message

Nikunj Kela July 18, 2023, 4:08 p.m. UTC
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.

Nikunj Kela (2):
  dt-bindings: arm: Add qcom specific hvc transport for SCMI
  firmware: arm_scmi: Add qcom hvc/shmem transport

 .../bindings/firmware/arm,scmi.yaml           |  69 +++++
 drivers/firmware/arm_scmi/Kconfig             |  13 +
 drivers/firmware/arm_scmi/Makefile            |   1 +
 drivers/firmware/arm_scmi/common.h            |   3 +
 drivers/firmware/arm_scmi/driver.c            |   4 +
 drivers/firmware/arm_scmi/qcom_hvc.c          | 241 ++++++++++++++++++
 6 files changed, 331 insertions(+)
 create mode 100644 drivers/firmware/arm_scmi/qcom_hvc.c

Comments

Sudeep Holla July 19, 2023, 10:39 a.m. UTC | #1
On Tue, Jul 18, 2023 at 09:08:32AM -0700, Nikunj Kela wrote:
> Introduce compatible "qcom,scmi-hvc-shmem" for SCMI
> transport channel for Qualcomm virtual platforms.
> The compatible mandates a shared memory channel.
> 
> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
> ---
>  .../bindings/firmware/arm,scmi.yaml           | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index b138f3d23df8..605b1e997a85 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -45,6 +45,9 @@ properties:
>        - description: SCMI compliant firmware with OP-TEE transport
>          items:
>            - const: linaro,scmi-optee
> +      - description: SCMI compliant firmware with Qualcomm hvc/shmem transport
> +        items:
> +          - const: qcom,scmi-hvc-shmem
>  
>    interrupts:
>      description:
> @@ -321,6 +324,16 @@ else:
>        required:
>          - linaro,optee-channel-id
>  
> +    else:
> +      if:
> +        properties:
> +          compatible:
> +            contains:
> +              const: qcom,scmi-hvc-shmem
> +      then:
> +        required:
> +          - shmem
> +

Since this was done after we merged the support recently for extension of
SMC/HVC, I need the reason(s) why this can't be used and based on the response
this is new change so it can't be because there is a product already
supporting this. So for now, NACK until I get the response for these.
Nikunj Kela July 19, 2023, 1:58 p.m. UTC | #2
On 7/19/2023 3:39 AM, Sudeep Holla wrote:
> On Tue, Jul 18, 2023 at 09:08:32AM -0700, Nikunj Kela wrote:
>> Introduce compatible "qcom,scmi-hvc-shmem" for SCMI
>> transport channel for Qualcomm virtual platforms.
>> The compatible mandates a shared memory channel.
>>
>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
>> ---
>>   .../bindings/firmware/arm,scmi.yaml           | 69 +++++++++++++++++++
>>   1 file changed, 69 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>> index b138f3d23df8..605b1e997a85 100644
>> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>> @@ -45,6 +45,9 @@ properties:
>>         - description: SCMI compliant firmware with OP-TEE transport
>>           items:
>>             - const: linaro,scmi-optee
>> +      - description: SCMI compliant firmware with Qualcomm hvc/shmem transport
>> +        items:
>> +          - const: qcom,scmi-hvc-shmem
>>   
>>     interrupts:
>>       description:
>> @@ -321,6 +324,16 @@ else:
>>         required:
>>           - linaro,optee-channel-id
>>   
>> +    else:
>> +      if:
>> +        properties:
>> +          compatible:
>> +            contains:
>> +              const: qcom,scmi-hvc-shmem
>> +      then:
>> +        required:
>> +          - shmem
>> +
> Since this was done after we merged the support recently for extension of
> SMC/HVC, I need the reason(s) why this can't be used and based on the response
> this is new change so it can't be because there is a product already
> supporting this. So for now, NACK until I get the response for these.
Our hypervisor deals with objects and uses object-ids to identify them. 
The hvc doorbell object is independent of the shmem object created by 
hypervisor. Hypervisor treats them independently. With the patch you 
mentioned, hypervisor need to create an association between the doorbell 
object and shmem object which will make it SCMI specific change in 
hypervisor. Hypervisor doesn't really care if doorbell is for SCMI or 
for other purposes hence we are adding this new driver so it can work 
with our hypervisor ABIs specification.
Konrad Dybcio Sept. 7, 2023, 4:16 p.m. UTC | #3
On 18.07.2023 18:08, 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.
What can we test it on?

Konrad
Nikunj Kela Sept. 7, 2023, 10:32 p.m. UTC | #4
On 9/7/2023 9:16 AM, Konrad Dybcio wrote:
> On 18.07.2023 18:08, 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.
> What can we test it on?
>
> Konrad
This is being developed for SA8775p platform.
Brian Masney Oct. 6, 2023, 8:17 p.m. UTC | #5
On Fri, Oct 06, 2023 at 09:42:06AM -0700, Nikunj Kela wrote:
> This change adds the support for SCMI message exchange on Qualcomm
> virtual platforms.
> 
> The hypervisor associates an object-id also known as capability-id
> with each smc/hvc doorbell object. The capability-id is used to
> identify the doorbell from the VM's capability namespace, similar
> to a file-descriptor.
> 
> The hypervisor, in addition to the function-id, expects the capability-id
> to be passed in x1 register when SMC/HVC call is invoked.
> 
> The capability-id is allocated by the hypervisor on bootup and is stored in
> the shmem region by the firmware before starting Linux.
> 
> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>

Reviewed-by: Brian Masney <bmasney@redhat.com>
Nikunj Kela Oct. 9, 2023, 7:14 p.m. UTC | #6
This change augments smc transport to include support for Qualcomm virtual
platforms by passing a parameter(capability-id) 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 capability-id is stored by firmware in the shmem region.

This has been tested on ARM64 virtual Qualcomm platform.

---
v6 -> use unsigned long for cap-id

v5 -> changed compatible, removed polling support patch,
      make use of smc-id binding for function-id

v4 -> port the changes into smc.c

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 (2):
  dt-bindings: arm: Add new compatible for smc/hvc transport for SCMI
  firmware: arm_scmi: Add qcom smc/hvc transport support

 .../bindings/firmware/arm,scmi.yaml           |  4 +++
 drivers/firmware/arm_scmi/driver.c            |  1 +
 drivers/firmware/arm_scmi/smc.c               | 27 +++++++++++++++++--
 3 files changed, 30 insertions(+), 2 deletions(-)
Sudeep Holla Oct. 10, 2023, 10:21 a.m. UTC | #7
On Mon, 09 Oct 2023 12:14:35 -0700, Nikunj Kela wrote:
> This change augments smc transport to include support for Qualcomm virtual
> platforms by passing a parameter(capability-id) 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 capability-id is stored by firmware in the shmem region.
>
> [...]

Applied to sudeep.holla/linux (for-next/scmi/updates), thanks!

[1/2] dt-bindings: arm: Add new compatible for smc/hvc transport for SCMI
      https://git.kernel.org/sudeep.holla/c/6979f88f5a8e
[2/2] firmware: arm_scmi: Add qcom smc/hvc transport support
      https://git.kernel.org/sudeep.holla/c/da405477e767
--
Regards,
Sudeep