Messages in this thread | | | Date | Tue, 6 Feb 2024 12:28:27 +0000 | Subject | Re: [Patch] memory: tegra: Skip SID override from Guest VM | From | Jon Hunter <> |
| |
Hi Marc,
On 06/02/2024 12:17, Marc Zyngier wrote: > Hi Sumit, > > On Tue, 06 Feb 2024 11:48:52 +0000, > Sumit Gupta <sumitg@nvidia.com> wrote: >> >> MC SID register access is restricted for Guest VM. >> So, skip the SID override programming from the Guest VM. >> >> Signed-off-by: Sumit Gupta <sumitg@nvidia.com> >> --- >> drivers/memory/tegra/tegra186.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c >> index 1b3183951bfe..df441896b69d 100644 >> --- a/drivers/memory/tegra/tegra186.c >> +++ b/drivers/memory/tegra/tegra186.c >> @@ -10,6 +10,7 @@ >> #include <linux/of.h> >> #include <linux/of_platform.h> >> #include <linux/platform_device.h> >> +#include <asm/virt.h> >> >> #include <soc/tegra/mc.h> >> >> @@ -118,6 +119,11 @@ static int tegra186_mc_probe_device(struct tegra_mc *mc, struct device *dev) >> unsigned int i, index = 0; >> u32 sid; >> >> + if (!is_kernel_in_hyp_mode()) { >> + dev_dbg(mc->dev, "Register access not allowed\n"); >> + return 0; >> + } >> + >> if (!tegra_dev_iommu_get_stream_id(dev, &sid)) >> return 0; >> >> @@ -146,6 +152,11 @@ static int tegra186_mc_resume(struct tegra_mc *mc) >> #if IS_ENABLED(CONFIG_IOMMU_API) >> unsigned int i; >> >> + if (!is_kernel_in_hyp_mode()) { >> + dev_dbg(mc->dev, "Register access not allowed\n"); >> + return 0; >> + } >> + >> for (i = 0; i < mc->soc->num_clients; i++) { >> const struct tegra_mc_client *client = &mc->soc->clients[i]; >> > > This doesn't look right. Multiple reasons: > > - This helper really has nothing to do in a driver. This is > architectural stuff that is not intended for use outside of arch > core code.
We see a few other drivers using this ...
drivers/perf/arm_pmuv3.c drivers/clocksource/arm_arch_timer.c drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h drivers/hwtracing/coresight/coresight-etm4x-core.c drivers/hwtracing/coresight/coresight-etm4x-core.c drivers/irqchip/irq-apple-aic.c
We were looking for a way to determine if the OS is a guest OS or not. However, I can see that this is a ARM64 specific API and so probably the above are only compiled for ARM64. Interestingly, the AMD driver implements the following ...
static inline bool is_virtual_machine(void) { #if defined(CONFIG_X86) return boot_cpu_has(X86_FEATURE_HYPERVISOR); #elif defined(CONFIG_ARM64) return !is_kernel_in_hyp_mode(); #else return false; #endif }
> - My own tegra186 HW doesn't have VHE, since it is ARMv8.0, and this > helper will always return 'false'. How could this result in > something that still works? Can I get a free CPU upgrade?
I thought this API just checks to see if we are in EL2?
> - If you assign this device to a VM and that the hypervisor doesn't > correctly virtualise it, then it is a different device and you > should simply advertise it something else. Or even better, fix your > hypervisor.
Sumit can add some more details on why we don't completely disable the device for guest OSs.
Jon
-- nvpublic
| |