Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Passthrough NVIDIA Jetson AGX Orin WiFi card to NetVM #93

Closed
wants to merge 2 commits into from
Closed

Passthrough NVIDIA Jetson AGX Orin WiFi card to NetVM #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
284b9f3
WIP: Use systemd-boot to load devicetree
Feb 21, 2023
648b353
Passthrough Jetson Orin WiFi card to microvm
panufi Mar 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions microvmConfigurations/netvm/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ nixpkgs.lib.nixosSystem {
# For WLAN firmwares
hardware.enableRedistributableFirmware = true;

microvm.hypervisor = "crosvm";
# TODO: change back to crosvm after tested working
microvm.hypervisor = "qemu";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the situation with the crosvm currently?


networking.enableIPv6 = false;
networking.interfaces.eth0.useDHCP = true;
Expand All @@ -39,16 +40,12 @@ nixpkgs.lib.nixosSystem {
# path = "vendorid=0x050d,productid=0x2103";
# }
# ];
# microvm.devices = [
# {
# bus = "pci";
# path = "0001:00:00.0";
# }
# {
# bus = "pci";
# path = "0001:01:00.0";
# }
# ];
microvm.devices = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to have some toggle for this, and not enable this always by default. Also this change is NVIDIA specific but affects all other targets as well. Of course this is a bigger refactoring question that might not be addressed within the context of this PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's valid toggle.

{
bus = "pci";
path = "0001:01:00.0";
}
];

# TODO: Move to user specified module - depending on the use x86_64
# laptop pci path
Expand Down
32 changes: 28 additions & 4 deletions modules/hardware/nvidia-jetson-orin.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Copyright 2022-2023 TII (SSRC) and the Ghaf contributors
# SPDX-License-Identifier: Apache-2.0
{...}: {
{config, ...}: {
hardware.nvidia-jetpack = {
enable = true;
som = "orin-agx";
Expand All @@ -10,8 +10,32 @@

nixpkgs.hostPlatform.system = "aarch64-linux";

boot.loader = {
efi.canTouchEfiVariables = true;
systemd-boot.enable = true;
boot.kernelPatches = [
{
name = "passthrough-patch";
patch = ./pci-passthrough-test.patch;
}
];

hardware.deviceTree = {
enable = true;
name = "tegra234-p3701-host-passthrough.dtb";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like this to be an overlay, instead of defining completely new devicetree that replaces the old

};

# Passthrough Jetson Orin WiFi card
boot.kernelParams = [
"vfio-pci" "ids=10ec:c82f"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to have some toggle for this, and not enable this always by default

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the motivation on Orin reference device? Wifi is basically the only networking interface we can easily and consistently pass through to netvm. I'd rather have it enabled by default even when we can't/shouldn't make it connect any WIFI automatically. And to support wifi, bring nmcli to netvm to easily scan and connect wifis over debug console.

"vfio_iommu_type1.allow_unsafe_interrupts=1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allow_unsafe_interrupts enables threat vector via MSI (message signaled interrupts). This is possible with PCI devices if there's access to device configuration space. See https://invisiblethingslab.com/resources/2011/Software%20Attacks%20on%20Intel%20VT-d.pdf - pages 6-7.

I'll add this note to acknowledge the threat from within the netvm and make a ghaf issue out of this to document it - at least until we have iommu group interrupt remapping support or other documented VMM mitigations. It may be that there's other mitigations these days that I'm not aware of. If that turns out to be the case, let's document those and close the issue after the fact.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we already had some discussion about this, and in the discussion the conclusion was that this allow_unsafe_interrupts is not needed. But I'm not sure did anyone test with this line removed

];

boot.loader.efi.canTouchEfiVariables = true;
boot.loader.systemd-boot = {
enable = true;
# TODO: Maybe add store path or some unique identifier to the filename
extraFiles."dtbs/${config.hardware.deviceTree.name}" = "${config.hardware.deviceTree.package}/${config.hardware.deviceTree.name}";
extraInstallCommands = ''
default_cfg=$(cat /boot/loader/loader.conf | grep default | awk '{print $2}')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stuff is now available in module boot/systemd-boot-dtb.nix. This whole WIP commit can be removed from this branch

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to push @panufi 's PR branch from his personal fork to under ghaf? We could then work many of these issues together in the same branch while he is out. Including rebasing with main to get some of the microvm.nix upstream changes merged to main earlier.

echo "devicetree /dtbs/${config.hardware.deviceTree.name}" >> /boot/loader/entries/$default_cfg
'';
};
}
32 changes: 32 additions & 0 deletions modules/hardware/pci-passthrough-test.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
diff --git a/nvidia/platform/t23x/concord/kernel-dts/Makefile b/nvidia/platform/t23x/concord/kernel-dts/Makefile
index 1be5b3f76bf8..01d3dea90cb5 100644
--- a/nvidia/platform/t23x/concord/kernel-dts/Makefile
+++ b/nvidia/platform/t23x/concord/kernel-dts/Makefile
@@ -23,6 +23,9 @@ dtb-$(BUILD_ENABLE) += tegra234-p3701-0000-as-p3767-0001-p3737-0000.dtb
dtb-$(BUILD_ENABLE) += tegra234-p3701-0000-as-pxxxx-p3737-0000.dtb
dtb-$(BUILD_ENABLE) += tegra234-p3701-0000-p3737-0000-kexec.dtb
dtb-$(BUILD_ENABLE) += tegra234-p3701-0004-p3737-0000.dtb
+
+dtb-$(BUILD_ENABLE) += tegra234-p3701-host-passthrough.dtb
+
dtbo-$(BUILD_ENABLE) += tegra234-p3737-a03-overlay.dtbo
dtbo-$(BUILD_ENABLE) += tegra234-p3737-a04-overlay.dtbo
dtbo-$(BUILD_ENABLE) += tegra234-p3737-overlay-pcie.dtbo
diff --git a/nvidia/platform/t23x/concord/kernel-dts/tegra234-p3701-host-passthrough.dts b/nvidia/platform/t23x/concord/kernel-dts/tegra234-p3701-host-passthrough.dts
new file mode 100644
index 000000000000..e4656287da82
--- /dev/null
+++ b/nvidia/platform/t23x/concord/kernel-dts/tegra234-p3701-host-passthrough.dts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like this to be an overlay, instead of defining completely new devicetree that replaces the old

@@ -0,0 +1,12 @@
+/dts-v1/;
+#include "tegra234-p3701-0000-p3737-0000.dts"
+
+/*
+ * Update the pci-e wifi to be accessible from vfio/guest
+ */
+&pcie_c1_rp {
+ interconnect-names = "dma-mem", "write";
+ /delete-property/ iommus;
+ /delete-property/ msi-parent;
+ /delete-property/ msi-map;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous concern with MSI seems irrelevant as we bypass the IOMMU protections completely for now. As this may be temporary but enables functional passthrough, I'll leave the earlier gh issue open and link this there.

+};
3 changes: 3 additions & 0 deletions modules/host/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,7 @@

networking.hostName = "ghaf-host";
system.stateVersion = "22.11";

# PCI passthrough needs larger locked-in-memory space than default
systemd.services."microvm@".serviceConfig.LimitMEMLOCK = 999999999;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line breaks the build

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was already fixed in microvm.nix astro/microvm.nix@68a0242

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll wait for @panufi 's comment on allow_unsafe_interrupts before testing removing it.

}
6 changes: 6 additions & 0 deletions modules/host/networking.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@
firewall.allowedUDPPorts = [67]; # DHCP
useNetworkd = true;
};

networking.nat = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address host NAT in the minimal host documentation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This enables NAT from host to the VMs, i.e. you can share the host's network with VMs. This is not needed at all other than debugging purposes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. This should go only via nix development-modules then. Even if I'd like to see "no networking on host" in development mode as well, it may not be very practical at this phase in development when tools from nix caches are often quite helpful to try/debug something in the development. Making the host no-networking/read-only (like netvm) would require reflashing for any small change.

enable = true;
internalInterfaces = [ "virbr0" ];
};

systemd.network = {
netdevs."virbr0".netdevConfig = {
Kind = "bridge";
Expand Down