2021-11-08 15:34:06

by Mikko Perttunen

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] Host1x context isolation support

On 9/16/21 5:32 PM, Mikko Perttunen wrote:
> Hi all,
>
> ***
> New in v2:
>
> Added support for Tegra194
> Use standard iommu-map property instead of custom mechanism
> ***
>
> this series adds support for Host1x 'context isolation'. Since
> when programming engines through Host1x, userspace can program in
> any addresses it wants, we need some way to isolate the engines'
> memory spaces. Traditionally this has either been done imperfectly
> with a single shared IOMMU domain, or by copying and verifying the
> programming command stream at submit time (Host1x firewall).
>
> Since Tegra186 there is a privileged (only usable by kernel)
> Host1x opcode that allows setting the stream ID sent by the engine
> to the SMMU. So, by allocating a number of context banks and stream
> IDs for this purpose, and using this opcode at the beginning of
> each job, we can implement isolation. Due to the limited number of
> context banks only each process gets its own context, and not
> each channel.
>
> This feature also allows sharing engines among multiple VMs when
> used with Host1x's hardware virtualization support - up to 8 VMs
> can be configured with a subset of allowed stream IDs, enforced
> at hardware level.
>
> To implement this, this series adds a new host1x context bus, which
> will contain the 'struct device's corresponding to each context
> bank / stream ID, changes to device tree and SMMU code to allow
> registering the devices and using the bus, as well as the Host1x
> stream ID programming code and support in TegraDRM.
>
> Device tree bindings are not updated yet pending consensus that the
> proposed changes make sense.
>
> Thanks,
> Mikko
>
> Mikko Perttunen (8):
> gpu: host1x: Add context bus
> gpu: host1x: Add context device management code
> gpu: host1x: Program context stream ID on submission
> iommu/arm-smmu: Attach to host1x context device bus
> arm64: tegra: Add Host1x context stream IDs on Tegra186+
> drm/tegra: falcon: Set DMACTX field on DMA transactions
> drm/tegra: vic: Implement get_streamid_offset
> drm/tegra: Support context isolation
>
> arch/arm64/boot/dts/nvidia/tegra186.dtsi | 12 ++
> arch/arm64/boot/dts/nvidia/tegra194.dtsi | 12 ++
> drivers/gpu/Makefile | 3 +-
> drivers/gpu/drm/tegra/drm.h | 2 +
> drivers/gpu/drm/tegra/falcon.c | 8 +
> drivers/gpu/drm/tegra/falcon.h | 1 +
> drivers/gpu/drm/tegra/submit.c | 13 ++
> drivers/gpu/drm/tegra/uapi.c | 34 ++++-
> drivers/gpu/drm/tegra/vic.c | 38 +++++
> drivers/gpu/host1x/Kconfig | 5 +
> drivers/gpu/host1x/Makefile | 2 +
> drivers/gpu/host1x/context.c | 174 ++++++++++++++++++++++
> drivers/gpu/host1x/context.h | 27 ++++
> drivers/gpu/host1x/context_bus.c | 31 ++++
> drivers/gpu/host1x/dev.c | 12 +-
> drivers/gpu/host1x/dev.h | 2 +
> drivers/gpu/host1x/hw/channel_hw.c | 52 ++++++-
> drivers/gpu/host1x/hw/host1x06_hardware.h | 10 ++
> drivers/gpu/host1x/hw/host1x07_hardware.h | 10 ++
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 ++
> include/linux/host1x.h | 21 +++
> include/linux/host1x_context_bus.h | 15 ++
> 22 files changed, 488 insertions(+), 9 deletions(-)
> create mode 100644 drivers/gpu/host1x/context.c
> create mode 100644 drivers/gpu/host1x/context.h
> create mode 100644 drivers/gpu/host1x/context_bus.c
> create mode 100644 include/linux/host1x_context_bus.h
>

IOMMU/DT folks, any thoughts about this approach? The patches that are
of interest outside of Host1x/TegraDRM specifics are patches 1, 2, 4, and 5.

Thanks,
Mikko


2021-12-06 09:55:27

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] Host1x context isolation support

Will, Joerg, Rob,

On 08/11/2021 10:36, Mikko Perttunen wrote:
> On 9/16/21 5:32 PM, Mikko Perttunen wrote:
>> Hi all,
>>
>> ***
>> New in v2:
>>
>> Added support for Tegra194
>> Use standard iommu-map property instead of custom mechanism
>> ***
>>
>> this series adds support for Host1x 'context isolation'. Since
>> when programming engines through Host1x, userspace can program in
>> any addresses it wants, we need some way to isolate the engines'
>> memory spaces. Traditionally this has either been done imperfectly
>> with a single shared IOMMU domain, or by copying and verifying the
>> programming command stream at submit time (Host1x firewall).
>>
>> Since Tegra186 there is a privileged (only usable by kernel)
>> Host1x opcode that allows setting the stream ID sent by the engine
>> to the SMMU. So, by allocating a number of context banks and stream
>> IDs for this purpose, and using this opcode at the beginning of
>> each job, we can implement isolation. Due to the limited number of
>> context banks only each process gets its own context, and not
>> each channel.
>>
>> This feature also allows sharing engines among multiple VMs when
>> used with Host1x's hardware virtualization support - up to 8 VMs
>> can be configured with a subset of allowed stream IDs, enforced
>> at hardware level.
>>
>> To implement this, this series adds a new host1x context bus, which
>> will contain the 'struct device's corresponding to each context
>> bank / stream ID, changes to device tree and SMMU code to allow
>> registering the devices and using the bus, as well as the Host1x
>> stream ID programming code and support in TegraDRM.
>>
>> Device tree bindings are not updated yet pending consensus that the
>> proposed changes make sense.
>>
>> Thanks,
>> Mikko
>>
>> Mikko Perttunen (8):
>>    gpu: host1x: Add context bus
>>    gpu: host1x: Add context device management code
>>    gpu: host1x: Program context stream ID on submission
>>    iommu/arm-smmu: Attach to host1x context device bus
>>    arm64: tegra: Add Host1x context stream IDs on Tegra186+
>>    drm/tegra: falcon: Set DMACTX field on DMA transactions
>>    drm/tegra: vic: Implement get_streamid_offset
>>    drm/tegra: Support context isolation
>>
>>   arch/arm64/boot/dts/nvidia/tegra186.dtsi  |  12 ++
>>   arch/arm64/boot/dts/nvidia/tegra194.dtsi  |  12 ++
>>   drivers/gpu/Makefile                      |   3 +-
>>   drivers/gpu/drm/tegra/drm.h               |   2 +
>>   drivers/gpu/drm/tegra/falcon.c            |   8 +
>>   drivers/gpu/drm/tegra/falcon.h            |   1 +
>>   drivers/gpu/drm/tegra/submit.c            |  13 ++
>>   drivers/gpu/drm/tegra/uapi.c              |  34 ++++-
>>   drivers/gpu/drm/tegra/vic.c               |  38 +++++
>>   drivers/gpu/host1x/Kconfig                |   5 +
>>   drivers/gpu/host1x/Makefile               |   2 +
>>   drivers/gpu/host1x/context.c              | 174 ++++++++++++++++++++++
>>   drivers/gpu/host1x/context.h              |  27 ++++
>>   drivers/gpu/host1x/context_bus.c          |  31 ++++
>>   drivers/gpu/host1x/dev.c                  |  12 +-
>>   drivers/gpu/host1x/dev.h                  |   2 +
>>   drivers/gpu/host1x/hw/channel_hw.c        |  52 ++++++-
>>   drivers/gpu/host1x/hw/host1x06_hardware.h |  10 ++
>>   drivers/gpu/host1x/hw/host1x07_hardware.h |  10 ++
>>   drivers/iommu/arm/arm-smmu/arm-smmu.c     |  13 ++
>>   include/linux/host1x.h                    |  21 +++
>>   include/linux/host1x_context_bus.h        |  15 ++
>>   22 files changed, 488 insertions(+), 9 deletions(-)
>>   create mode 100644 drivers/gpu/host1x/context.c
>>   create mode 100644 drivers/gpu/host1x/context.h
>>   create mode 100644 drivers/gpu/host1x/context_bus.c
>>   create mode 100644 include/linux/host1x_context_bus.h
>>
>
> IOMMU/DT folks, any thoughts about this approach? The patches that are
> of interest outside of Host1x/TegraDRM specifics are patches 1, 2, 4,
> and 5.


Any feedback on this?

Jon

--
nvpublic

2021-12-14 08:06:03

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] Host1x context isolation support

Hi all,

Still no response on this :-(

On 06/12/2021 09:55, Jon Hunter wrote:
> Will, Joerg, Rob,
>
> On 08/11/2021 10:36, Mikko Perttunen wrote:
>> On 9/16/21 5:32 PM, Mikko Perttunen wrote:
>>> Hi all,
>>>
>>> ***
>>> New in v2:
>>>
>>> Added support for Tegra194
>>> Use standard iommu-map property instead of custom mechanism
>>> ***
>>>
>>> this series adds support for Host1x 'context isolation'. Since
>>> when programming engines through Host1x, userspace can program in
>>> any addresses it wants, we need some way to isolate the engines'
>>> memory spaces. Traditionally this has either been done imperfectly
>>> with a single shared IOMMU domain, or by copying and verifying the
>>> programming command stream at submit time (Host1x firewall).
>>>
>>> Since Tegra186 there is a privileged (only usable by kernel)
>>> Host1x opcode that allows setting the stream ID sent by the engine
>>> to the SMMU. So, by allocating a number of context banks and stream
>>> IDs for this purpose, and using this opcode at the beginning of
>>> each job, we can implement isolation. Due to the limited number of
>>> context banks only each process gets its own context, and not
>>> each channel.
>>>
>>> This feature also allows sharing engines among multiple VMs when
>>> used with Host1x's hardware virtualization support - up to 8 VMs
>>> can be configured with a subset of allowed stream IDs, enforced
>>> at hardware level.
>>>
>>> To implement this, this series adds a new host1x context bus, which
>>> will contain the 'struct device's corresponding to each context
>>> bank / stream ID, changes to device tree and SMMU code to allow
>>> registering the devices and using the bus, as well as the Host1x
>>> stream ID programming code and support in TegraDRM.
>>>
>>> Device tree bindings are not updated yet pending consensus that the
>>> proposed changes make sense.
>>>
>>> Thanks,
>>> Mikko
>>>
>>> Mikko Perttunen (8):
>>>    gpu: host1x: Add context bus
>>>    gpu: host1x: Add context device management code
>>>    gpu: host1x: Program context stream ID on submission
>>>    iommu/arm-smmu: Attach to host1x context device bus
>>>    arm64: tegra: Add Host1x context stream IDs on Tegra186+
>>>    drm/tegra: falcon: Set DMACTX field on DMA transactions
>>>    drm/tegra: vic: Implement get_streamid_offset
>>>    drm/tegra: Support context isolation
>>>
>>>   arch/arm64/boot/dts/nvidia/tegra186.dtsi  |  12 ++
>>>   arch/arm64/boot/dts/nvidia/tegra194.dtsi  |  12 ++
>>>   drivers/gpu/Makefile                      |   3 +-
>>>   drivers/gpu/drm/tegra/drm.h               |   2 +
>>>   drivers/gpu/drm/tegra/falcon.c            |   8 +
>>>   drivers/gpu/drm/tegra/falcon.h            |   1 +
>>>   drivers/gpu/drm/tegra/submit.c            |  13 ++
>>>   drivers/gpu/drm/tegra/uapi.c              |  34 ++++-
>>>   drivers/gpu/drm/tegra/vic.c               |  38 +++++
>>>   drivers/gpu/host1x/Kconfig                |   5 +
>>>   drivers/gpu/host1x/Makefile               |   2 +
>>>   drivers/gpu/host1x/context.c              | 174 ++++++++++++++++++++++
>>>   drivers/gpu/host1x/context.h              |  27 ++++
>>>   drivers/gpu/host1x/context_bus.c          |  31 ++++
>>>   drivers/gpu/host1x/dev.c                  |  12 +-
>>>   drivers/gpu/host1x/dev.h                  |   2 +
>>>   drivers/gpu/host1x/hw/channel_hw.c        |  52 ++++++-
>>>   drivers/gpu/host1x/hw/host1x06_hardware.h |  10 ++
>>>   drivers/gpu/host1x/hw/host1x07_hardware.h |  10 ++
>>>   drivers/iommu/arm/arm-smmu/arm-smmu.c     |  13 ++
>>>   include/linux/host1x.h                    |  21 +++
>>>   include/linux/host1x_context_bus.h        |  15 ++
>>>   22 files changed, 488 insertions(+), 9 deletions(-)
>>>   create mode 100644 drivers/gpu/host1x/context.c
>>>   create mode 100644 drivers/gpu/host1x/context.h
>>>   create mode 100644 drivers/gpu/host1x/context_bus.c
>>>   create mode 100644 include/linux/host1x_context_bus.h
>>>
>>
>> IOMMU/DT folks, any thoughts about this approach? The patches that are
>> of interest outside of Host1x/TegraDRM specifics are patches 1, 2, 4,
>> and 5.
>
>
> Any feedback on this?
>
> Jon
>

--
nvpublic

2021-12-14 14:35:44

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] Host1x context isolation support

14.12.2021 11:05, Jon Hunter пишет:
> Hi all,
>
> Still no response on this :-(

I see only two patches on Tegra ML and others on DRI ML. Might be good
to start with re-sending this whole series and CCing MLs properly.


2021-12-14 14:53:31

by Mikko Perttunen

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] Host1x context isolation support

On 12/14/21 16:35, Dmitry Osipenko wrote:
> 14.12.2021 11:05, Jon Hunter пишет:
>> Hi all,
>>
>> Still no response on this :-(
>
> I see only two patches on Tegra ML and others on DRI ML. Might be good
> to start with re-sending this whole series and CCing MLs properly.
>

All patches should have been sent to the same set of addresses. At least
LWN's archive seems to agree..

Mikko

2021-12-14 15:31:22

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] Host1x context isolation support

14.12.2021 17:53, Mikko Perttunen пишет:
> On 12/14/21 16:35, Dmitry Osipenko wrote:
>> 14.12.2021 11:05, Jon Hunter пишет:
>>> Hi all,
>>>
>>> Still no response on this :-(
>>
>> I see only two patches on Tegra ML and others on DRI ML. Might be good
>> to start with re-sending this whole series and CCing MLs properly.
>>
>
> All patches should have been sent to the same set of addresses. At least
> LWN's archive seems to agree..

Indeed, I see that Tegra ML was CCed and I see all patches on Tegra
patchwork, but I don't see them all on lore and gmane.

2021-12-14 15:39:03

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] Host1x context isolation support

On 2021-11-08 10:36, Mikko Perttunen wrote:
> On 9/16/21 5:32 PM, Mikko Perttunen wrote:
>> Hi all,
>>
>> ***
>> New in v2:
>>
>> Added support for Tegra194
>> Use standard iommu-map property instead of custom mechanism
>> ***
>>
>> this series adds support for Host1x 'context isolation'. Since
>> when programming engines through Host1x, userspace can program in
>> any addresses it wants, we need some way to isolate the engines'
>> memory spaces. Traditionally this has either been done imperfectly
>> with a single shared IOMMU domain, or by copying and verifying the
>> programming command stream at submit time (Host1x firewall).
>>
>> Since Tegra186 there is a privileged (only usable by kernel)
>> Host1x opcode that allows setting the stream ID sent by the engine
>> to the SMMU. So, by allocating a number of context banks and stream
>> IDs for this purpose, and using this opcode at the beginning of
>> each job, we can implement isolation. Due to the limited number of
>> context banks only each process gets its own context, and not
>> each channel.
>>
>> This feature also allows sharing engines among multiple VMs when
>> used with Host1x's hardware virtualization support - up to 8 VMs
>> can be configured with a subset of allowed stream IDs, enforced
>> at hardware level.
>>
>> To implement this, this series adds a new host1x context bus, which
>> will contain the 'struct device's corresponding to each context
>> bank / stream ID, changes to device tree and SMMU code to allow
>> registering the devices and using the bus, as well as the Host1x
>> stream ID programming code and support in TegraDRM.
>>
>> Device tree bindings are not updated yet pending consensus that the
>> proposed changes make sense.
>>
>> Thanks,
>> Mikko
>>
>> Mikko Perttunen (8):
>>    gpu: host1x: Add context bus
>>    gpu: host1x: Add context device management code
>>    gpu: host1x: Program context stream ID on submission
>>    iommu/arm-smmu: Attach to host1x context device bus
>>    arm64: tegra: Add Host1x context stream IDs on Tegra186+
>>    drm/tegra: falcon: Set DMACTX field on DMA transactions
>>    drm/tegra: vic: Implement get_streamid_offset
>>    drm/tegra: Support context isolation
>>
>>   arch/arm64/boot/dts/nvidia/tegra186.dtsi  |  12 ++
>>   arch/arm64/boot/dts/nvidia/tegra194.dtsi  |  12 ++
>>   drivers/gpu/Makefile                      |   3 +-
>>   drivers/gpu/drm/tegra/drm.h               |   2 +
>>   drivers/gpu/drm/tegra/falcon.c            |   8 +
>>   drivers/gpu/drm/tegra/falcon.h            |   1 +
>>   drivers/gpu/drm/tegra/submit.c            |  13 ++
>>   drivers/gpu/drm/tegra/uapi.c              |  34 ++++-
>>   drivers/gpu/drm/tegra/vic.c               |  38 +++++
>>   drivers/gpu/host1x/Kconfig                |   5 +
>>   drivers/gpu/host1x/Makefile               |   2 +
>>   drivers/gpu/host1x/context.c              | 174 ++++++++++++++++++++++
>>   drivers/gpu/host1x/context.h              |  27 ++++
>>   drivers/gpu/host1x/context_bus.c          |  31 ++++
>>   drivers/gpu/host1x/dev.c                  |  12 +-
>>   drivers/gpu/host1x/dev.h                  |   2 +
>>   drivers/gpu/host1x/hw/channel_hw.c        |  52 ++++++-
>>   drivers/gpu/host1x/hw/host1x06_hardware.h |  10 ++
>>   drivers/gpu/host1x/hw/host1x07_hardware.h |  10 ++
>>   drivers/iommu/arm/arm-smmu/arm-smmu.c     |  13 ++
>>   include/linux/host1x.h                    |  21 +++
>>   include/linux/host1x_context_bus.h        |  15 ++
>>   22 files changed, 488 insertions(+), 9 deletions(-)
>>   create mode 100644 drivers/gpu/host1x/context.c
>>   create mode 100644 drivers/gpu/host1x/context.h
>>   create mode 100644 drivers/gpu/host1x/context_bus.c
>>   create mode 100644 include/linux/host1x_context_bus.h
>>
>
> IOMMU/DT folks, any thoughts about this approach? The patches that are
> of interest outside of Host1x/TegraDRM specifics are patches 1, 2, 4,
> and 5.

FWIW it looks fairly innocuous to me. I don't understand host1x -
neither hardware nor driver abstractions - well enough to meaningfully
review it all (e.g. maybe it's deliberate that the bus .dma_configure
method isn't used?), but the SMMU patch seems fine given the Kconfig
solution to avoid module linkage problems.

Cheers,
Robin.

2021-12-17 11:16:22

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] Host1x context isolation support


On 14/12/2021 15:38, Robin Murphy wrote:

...

>> IOMMU/DT folks, any thoughts about this approach? The patches that are
>> of interest outside of Host1x/TegraDRM specifics are patches 1, 2, 4,
>> and 5.
>
> FWIW it looks fairly innocuous to me. I don't understand host1x -
> neither hardware nor driver abstractions - well enough to meaningfully
> review it all (e.g. maybe it's deliberate that the bus .dma_configure
> method isn't used?), but the SMMU patch seems fine given the Kconfig
> solution to avoid module linkage problems.


Thanks Robin! Will, Joerg, is OK with you?

Mikko, I believe we are missing a dt-binding change to document the
'memory-contexts' node which I assume you will add if everyone is OK
with this?

Cheers
Jon

--
nvpublic