| Message ID | 20220218113952.3077606-3-mperttunen@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Host1x context isolation support | expand |
18.02.2022 14:39, Mikko Perttunen пишет: > +config TEGRA_HOST1X_CONTEXT_BUS > + bool > + > config TEGRA_HOST1X > tristate "NVIDIA Tegra host1x driver" > depends on ARCH_TEGRA || (ARM && COMPILE_TEST) > select DMA_SHARED_BUFFER > + select TEGRA_HOST1X_CONTEXT_BUS What is the point of TEGRA_HOST1X_CONTEXT_BUS if it's selected unconditionally?
19.02.2022 20:54, Dmitry Osipenko пишет: > 18.02.2022 14:39, Mikko Perttunen пишет: >> +config TEGRA_HOST1X_CONTEXT_BUS >> + bool >> + >> config TEGRA_HOST1X >> tristate "NVIDIA Tegra host1x driver" >> depends on ARCH_TEGRA || (ARM && COMPILE_TEST) >> select DMA_SHARED_BUFFER >> + select TEGRA_HOST1X_CONTEXT_BUS > > What is the point of TEGRA_HOST1X_CONTEXT_BUS if it's selected > unconditionally? I see now that it's used by arm-smmu.c, should be okay then.
On Fri, Feb 18, 2022 at 01:39:45PM +0200, Mikko Perttunen wrote: > The context bus is a "dummy" bus that contains struct devices that > correspond to IOMMU contexts assigned through Host1x to processes. > > Even when host1x itself is built as a module, the bus is registered > in built-in code so that the built-in ARM SMMU driver is able to > reference it. > > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com> > --- > drivers/gpu/Makefile | 3 +-- > drivers/gpu/host1x/Kconfig | 5 +++++ > drivers/gpu/host1x/Makefile | 1 + > drivers/gpu/host1x/context_bus.c | 31 ++++++++++++++++++++++++++++++ > include/linux/host1x_context_bus.h | 15 +++++++++++++++ > 5 files changed, 53 insertions(+), 2 deletions(-) > create mode 100644 drivers/gpu/host1x/context_bus.c > create mode 100644 include/linux/host1x_context_bus.h > > diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile > index 835c88318cec..8997f0096545 100644 > --- a/drivers/gpu/Makefile > +++ b/drivers/gpu/Makefile > @@ -2,7 +2,6 @@ > # drm/tegra depends on host1x, so if both drivers are built-in care must be > # taken to initialize them in the correct order. Link order is the only way > # to ensure this currently. > -obj-$(CONFIG_TEGRA_HOST1X) += host1x/ > -obj-y += drm/ vga/ > +obj-y += host1x/ drm/ vga/ > obj-$(CONFIG_IMX_IPUV3_CORE) += ipu-v3/ > obj-$(CONFIG_TRACE_GPU_MEM) += trace/ > diff --git a/drivers/gpu/host1x/Kconfig b/drivers/gpu/host1x/Kconfig > index 6815b4db17c1..1861a8180d3f 100644 > --- a/drivers/gpu/host1x/Kconfig > +++ b/drivers/gpu/host1x/Kconfig > @@ -1,8 +1,13 @@ > # SPDX-License-Identifier: GPL-2.0-only > + > +config TEGRA_HOST1X_CONTEXT_BUS > + bool > + > config TEGRA_HOST1X > tristate "NVIDIA Tegra host1x driver" > depends on ARCH_TEGRA || (ARM && COMPILE_TEST) > select DMA_SHARED_BUFFER > + select TEGRA_HOST1X_CONTEXT_BUS > select IOMMU_IOVA > help > Driver for the NVIDIA Tegra host1x hardware. > diff --git a/drivers/gpu/host1x/Makefile b/drivers/gpu/host1x/Makefile > index d2b6f7de0498..c891a3e33844 100644 > --- a/drivers/gpu/host1x/Makefile > +++ b/drivers/gpu/host1x/Makefile > @@ -18,3 +18,4 @@ host1x-y = \ > hw/host1x07.o > > obj-$(CONFIG_TEGRA_HOST1X) += host1x.o > +obj-$(CONFIG_TEGRA_HOST1X_CONTEXT_BUS) += context_bus.o > diff --git a/drivers/gpu/host1x/context_bus.c b/drivers/gpu/host1x/context_bus.c > new file mode 100644 > index 000000000000..2625914f3c7d > --- /dev/null > +++ b/drivers/gpu/host1x/context_bus.c > @@ -0,0 +1,31 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2021, NVIDIA Corporation. > + */ > + > +#include <linux/device.h> > +#include <linux/of.h> > + > +struct bus_type host1x_context_device_bus_type = { > + .name = "host1x-context", > +}; > +EXPORT_SYMBOL(host1x_context_device_bus_type); EXPORT_SYMBOL_GPL, please. But the pattern that this copies in arm_smmu_bus_init is really ugly. I think we need to figure out a way todo that without having to export all the low-level bus types.
On 2022-02-22 16:21, Christoph Hellwig wrote: > On Fri, Feb 18, 2022 at 01:39:45PM +0200, Mikko Perttunen wrote: >> The context bus is a "dummy" bus that contains struct devices that >> correspond to IOMMU contexts assigned through Host1x to processes. >> >> Even when host1x itself is built as a module, the bus is registered >> in built-in code so that the built-in ARM SMMU driver is able to >> reference it. >> >> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com> >> --- >> drivers/gpu/Makefile | 3 +-- >> drivers/gpu/host1x/Kconfig | 5 +++++ >> drivers/gpu/host1x/Makefile | 1 + >> drivers/gpu/host1x/context_bus.c | 31 ++++++++++++++++++++++++++++++ >> include/linux/host1x_context_bus.h | 15 +++++++++++++++ >> 5 files changed, 53 insertions(+), 2 deletions(-) >> create mode 100644 drivers/gpu/host1x/context_bus.c >> create mode 100644 include/linux/host1x_context_bus.h >> >> diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile >> index 835c88318cec..8997f0096545 100644 >> --- a/drivers/gpu/Makefile >> +++ b/drivers/gpu/Makefile >> @@ -2,7 +2,6 @@ >> # drm/tegra depends on host1x, so if both drivers are built-in care must be >> # taken to initialize them in the correct order. Link order is the only way >> # to ensure this currently. >> -obj-$(CONFIG_TEGRA_HOST1X) += host1x/ >> -obj-y += drm/ vga/ >> +obj-y += host1x/ drm/ vga/ >> obj-$(CONFIG_IMX_IPUV3_CORE) += ipu-v3/ >> obj-$(CONFIG_TRACE_GPU_MEM) += trace/ >> diff --git a/drivers/gpu/host1x/Kconfig b/drivers/gpu/host1x/Kconfig >> index 6815b4db17c1..1861a8180d3f 100644 >> --- a/drivers/gpu/host1x/Kconfig >> +++ b/drivers/gpu/host1x/Kconfig >> @@ -1,8 +1,13 @@ >> # SPDX-License-Identifier: GPL-2.0-only >> + >> +config TEGRA_HOST1X_CONTEXT_BUS >> + bool >> + >> config TEGRA_HOST1X >> tristate "NVIDIA Tegra host1x driver" >> depends on ARCH_TEGRA || (ARM && COMPILE_TEST) >> select DMA_SHARED_BUFFER >> + select TEGRA_HOST1X_CONTEXT_BUS >> select IOMMU_IOVA >> help >> Driver for the NVIDIA Tegra host1x hardware. >> diff --git a/drivers/gpu/host1x/Makefile b/drivers/gpu/host1x/Makefile >> index d2b6f7de0498..c891a3e33844 100644 >> --- a/drivers/gpu/host1x/Makefile >> +++ b/drivers/gpu/host1x/Makefile >> @@ -18,3 +18,4 @@ host1x-y = \ >> hw/host1x07.o >> >> obj-$(CONFIG_TEGRA_HOST1X) += host1x.o >> +obj-$(CONFIG_TEGRA_HOST1X_CONTEXT_BUS) += context_bus.o >> diff --git a/drivers/gpu/host1x/context_bus.c b/drivers/gpu/host1x/context_bus.c >> new file mode 100644 >> index 000000000000..2625914f3c7d >> --- /dev/null >> +++ b/drivers/gpu/host1x/context_bus.c >> @@ -0,0 +1,31 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2021, NVIDIA Corporation. >> + */ >> + >> +#include <linux/device.h> >> +#include <linux/of.h> >> + >> +struct bus_type host1x_context_device_bus_type = { >> + .name = "host1x-context", >> +}; >> +EXPORT_SYMBOL(host1x_context_device_bus_type); > > EXPORT_SYMBOL_GPL, please. > > But the pattern that this copies in arm_smmu_bus_init is really > ugly. I think we need to figure out a way todo that without having > to export all the low-level bus types. Yup, as it happens that was the first step on my mission :) https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu/bus Still a way to go with the main meat of that work, though, so I was figuring this could probably land as-is and I'll sweep it up in due course. Robin.
On Tue, Feb 22, 2022 at 09:30:30PM +0000, Robin Murphy wrote: > > But the pattern that this copies in arm_smmu_bus_init is really > > ugly. I think we need to figure out a way todo that without having > > to export all the low-level bus types. > > Yup, as it happens that was the first step on my mission :) > > https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu/bus > > Still a way to go with the main meat of that work, though, so I was figuring > this could probably land as-is and I'll sweep it up in due course. Yes, that part looks very nice. It would be great to get that merged ASAP.
diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile index 835c88318cec..8997f0096545 100644 --- a/drivers/gpu/Makefile +++ b/drivers/gpu/Makefile @@ -2,7 +2,6 @@ # drm/tegra depends on host1x, so if both drivers are built-in care must be # taken to initialize them in the correct order. Link order is the only way # to ensure this currently. -obj-$(CONFIG_TEGRA_HOST1X) += host1x/ -obj-y += drm/ vga/ +obj-y += host1x/ drm/ vga/ obj-$(CONFIG_IMX_IPUV3_CORE) += ipu-v3/ obj-$(CONFIG_TRACE_GPU_MEM) += trace/ diff --git a/drivers/gpu/host1x/Kconfig b/drivers/gpu/host1x/Kconfig index 6815b4db17c1..1861a8180d3f 100644 --- a/drivers/gpu/host1x/Kconfig +++ b/drivers/gpu/host1x/Kconfig @@ -1,8 +1,13 @@ # SPDX-License-Identifier: GPL-2.0-only + +config TEGRA_HOST1X_CONTEXT_BUS + bool + config TEGRA_HOST1X tristate "NVIDIA Tegra host1x driver" depends on ARCH_TEGRA || (ARM && COMPILE_TEST) select DMA_SHARED_BUFFER + select TEGRA_HOST1X_CONTEXT_BUS select IOMMU_IOVA help Driver for the NVIDIA Tegra host1x hardware. diff --git a/drivers/gpu/host1x/Makefile b/drivers/gpu/host1x/Makefile index d2b6f7de0498..c891a3e33844 100644 --- a/drivers/gpu/host1x/Makefile +++ b/drivers/gpu/host1x/Makefile @@ -18,3 +18,4 @@ host1x-y = \ hw/host1x07.o obj-$(CONFIG_TEGRA_HOST1X) += host1x.o +obj-$(CONFIG_TEGRA_HOST1X_CONTEXT_BUS) += context_bus.o diff --git a/drivers/gpu/host1x/context_bus.c b/drivers/gpu/host1x/context_bus.c new file mode 100644 index 000000000000..2625914f3c7d --- /dev/null +++ b/drivers/gpu/host1x/context_bus.c @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2021, NVIDIA Corporation. + */ + +#include <linux/device.h> +#include <linux/of.h> + +struct bus_type host1x_context_device_bus_type = { + .name = "host1x-context", +}; +EXPORT_SYMBOL(host1x_context_device_bus_type); + +static int __init host1x_context_device_bus_init(void) +{ + int err; + + if (!of_machine_is_compatible("nvidia,tegra186") && + !of_machine_is_compatible("nvidia,tegra194") && + !of_machine_is_compatible("nvidia,tegra234")) + return 0; + + err = bus_register(&host1x_context_device_bus_type); + if (err < 0) { + pr_err("bus type registration failed: %d\n", err); + return err; + } + + return 0; +} +postcore_initcall(host1x_context_device_bus_init); diff --git a/include/linux/host1x_context_bus.h b/include/linux/host1x_context_bus.h new file mode 100644 index 000000000000..72462737a6db --- /dev/null +++ b/include/linux/host1x_context_bus.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (c) 2021, NVIDIA Corporation. All rights reserved. + */ + +#ifndef __LINUX_HOST1X_CONTEXT_BUS_H +#define __LINUX_HOST1X_CONTEXT_BUS_H + +#include <linux/device.h> + +#ifdef CONFIG_TEGRA_HOST1X_CONTEXT_BUS +extern struct bus_type host1x_context_device_bus_type; +#endif + +#endif
The context bus is a "dummy" bus that contains struct devices that correspond to IOMMU contexts assigned through Host1x to processes. Even when host1x itself is built as a module, the bus is registered in built-in code so that the built-in ARM SMMU driver is able to reference it. Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com> --- drivers/gpu/Makefile | 3 +-- drivers/gpu/host1x/Kconfig | 5 +++++ drivers/gpu/host1x/Makefile | 1 + drivers/gpu/host1x/context_bus.c | 31 ++++++++++++++++++++++++++++++ include/linux/host1x_context_bus.h | 15 +++++++++++++++ 5 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/host1x/context_bus.c create mode 100644 include/linux/host1x_context_bus.h