* [PATCH 1/2] add kconfig/ansible options for arm64 support
@ 2023-09-01 5:03 Ross Vandegrift
2023-09-01 5:03 ` [PATCH 2/2] add kconfig for opensuse arm64 vagrant images Ross Vandegrift
2023-09-01 19:57 ` [PATCH 1/2] add kconfig/ansible options for arm64 support Luis Chamberlain
0 siblings, 2 replies; 5+ messages in thread
From: Ross Vandegrift @ 2023-09-01 5:03 UTC (permalink / raw)
To: mcgrof; +Cc: kdevops, Ross Vandegrift
This adds new Kconfig options to support arm64 kdevops guests under vagrant +
libvirt. They are plumbed through to ansible, and make bringup works. A few
x86 specific things have had the dependency made explicit. I'm not sure I
found all of those things.
To test with this change, you need a local arm64 vagrant image. Debian's cloud
images support building them, but they aren't published yet. openSUSE defaults
will be added next.
Signed-off-by: Ross Vandegrift <ross@kallisti.us>
---
Debian's vagrant images are built/uploaded from this repo:
https://salsa.debian.org/cloud-team/debian-vagrant-images
The build supports multiple arches fine, but the upload piece does not yet. If
you want to build locally, it's roughly:
$ sudo make install-build-deps
$ make testing-vagrant-arm64
$ make build-libvirt-testing-vagrant-arm64
$ vagrant box add --name your-local-name libvirt-debian-testing-vagrant-arm64-official-20230831-1.box
---
kconfigs/Kconfig.bringup | 2 +-
playbooks/roles/gen_nodes/defaults/main.yml | 5 ++
.../roles/gen_nodes/templates/Vagrantfile.j2 | 15 ++++
.../kdevops_nodes_split_start.j2.yaml | 13 ++++
scripts/gen-nodes.Makefile | 24 ++++++
vagrant/Kconfig | 77 ++++++++++++++++++-
6 files changed, 132 insertions(+), 4 deletions(-)
diff --git a/kconfigs/Kconfig.bringup b/kconfigs/Kconfig.bringup
index 1eaa2c99..d559a877 100644
--- a/kconfigs/Kconfig.bringup
+++ b/kconfigs/Kconfig.bringup
@@ -5,7 +5,7 @@ choice
config VAGRANT
bool "Vagrant for local virtualization (KVM / VirtualBox)"
select KDEVOPS_SSH_CONFIG_UPDATE_STRICT
- depends on TARGET_ARCH_X86_64
+ depends on TARGET_ARCH_X86_64 || TARGET_ARCH_ARM64
help
This option will enable use of Vagrant. Enable this if you want to
use local virtualization technologies such as KVM with libvirt or
diff --git a/playbooks/roles/gen_nodes/defaults/main.yml b/playbooks/roles/gen_nodes/defaults/main.yml
index c57effe7..2cca984a 100644
--- a/playbooks/roles/gen_nodes/defaults/main.yml
+++ b/playbooks/roles/gen_nodes/defaults/main.yml
@@ -60,6 +60,11 @@ libvirt_extra_storage_aio_cache_mode: "none"
libvirt_extra_storage_nvme_logical_block_size: 512
libvirt_extra_storage_virtio_logical_block_size: 512
libvirt_extra_storage_virtio_physical_block_size: 512
+libvirt_input_mouse_bus: ps2
+libvirt_arm64_gic_version: "3"
+libvirt_graphics_type: vnc
+libvirt_loader: ""
+libvirt_nvram_dir: /var/lib/libvirt/qemu/nvram
libvirt_largeio_enable: False
libvirt_largeio_logical_compat: False
diff --git a/playbooks/roles/gen_nodes/templates/Vagrantfile.j2 b/playbooks/roles/gen_nodes/templates/Vagrantfile.j2
index 7ed59ff7..e064377b 100644
--- a/playbooks/roles/gen_nodes/templates/Vagrantfile.j2
+++ b/playbooks/roles/gen_nodes/templates/Vagrantfile.j2
@@ -229,6 +229,21 @@ Vagrant.configure("2") do |config|
libvirt.machine_type = global_data['libvirt_cfg']['machine_type']
end
end
+
+{% if libvirt_arm64_gic_version %}
+ libvirt.features = ["acpi", "gic version='%s'" % [global_data["libvirt_cfg"]['arm64_gic_version']]]
+{% endif %}
+{% if libvirt_graphics_type %}
+ libvirt.graphics_type = global_data["libvirt_cfg"]["graphics_type"]
+{% endif %}
+{% if libvirt_input_mouse_bus %}
+ libvirt.input :type => "mouse", :bus => global_data["libvirt_cfg"]["input_mouse_bus"]
+{% endif %}
+{% if libvirt_loader %}
+ libvirt.loader = global_data["libvirt_cfg"]["loader"]
+ libvirt.nvram = "%s/%s.fd" % [global_data["libvirt_cfg"]["nvram_dir"], host_name]
+{% endif %}
+
# Add an extra spare PCI or PCI-E root bus to be used for extra drives.
#
# We use a dedicated PCI or PCI-E root bus to not clash with defaults
diff --git a/playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml b/playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml
index 7cd95376..367990dc 100644
--- a/playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml
+++ b/playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml
@@ -24,6 +24,19 @@ vagrant_global:
libvirt_cfg:
{% if libvirt_override_machine_type %}
machine_type: '{{ libvirt_machine_type }}'
+{% endif %}
+{% if libvirt_arm64_gic_version %}
+ arm64_gic_version: {{ libvirt_arm64_gic_version }}
+{% endif %}
+{% if libvirt_graphics_type %}
+ graphics_type: {{ libvirt_graphics_type }}
+{% endif %}
+{% if libvirt_input_mouse_bus %}
+ input_mouse_bus: {{ libvirt_input_mouse_bus }}
+{% endif %}
+{% if libvirt_loader %}
+ loader: {{ libvirt_loader }}
+ nvram_dir: {{ libvirt_nvram_dir }}
{% endif %}
# This lets the Ansible role kdevops_vagrant try to infer your default
# distro group to use for QEMU. OpenSUSE and Fedora uses QEMU here by
diff --git a/scripts/gen-nodes.Makefile b/scripts/gen-nodes.Makefile
index 533b19db..5be1d1ba 100644
--- a/scripts/gen-nodes.Makefile
+++ b/scripts/gen-nodes.Makefile
@@ -203,4 +203,28 @@ endif # CONFIG_QEMU_ENABLE_CXL
endif # CONFIG_LIBVIRT_MACHINE_TYPE_Q35
+ifeq (y,$(CONFIG_LIBVIRT_MACHINE_TYPE_VIRT))
+GEN_NODES_EXTRA_ARGS += libvirt_override_machine_type='True'
+GEN_NODES_EXTRA_ARGS += libvirt_machine_type='virt'
+endif
+
+ifeq (y,$(CONFIG_LIBVIRT_INPUT_PS2_MOUSE))
+GEN_NODES_EXTRA_ARGS += libvirt_input_mouse_bus='ps2'
+else ifeq (y, $(CONFIG_LIBVIRT_INPUT_VIRTIO_MOUSE))
+GEN_NODES_EXTRA_ARGS += libvirt_input_mouse_bus='virtio'
+endif
+
+ifneq (,$(CONFIG_LIBVIRT_FEATURE_ARM64_GIC_VERSION))
+GEN_NODES_EXTRA_ARGS += libvirt_arm64_gic_version='$(subst ",,$(CONFIG_LIBVIRT_FEATURE_ARM64_GIC_VERSION))'
+endif
+
+ifneq (,$(CONFIG_LIBVIRT_GRAPHICS_TYPE))
+GEN_NODES_EXTRA_ARGS += libvirt_graphics_type='$(subst ",,$(CONFIG_LIBCIRT_GRAPHICS_TYPE))'
+endif
+
+ifneq (,$(CONFIG_LIBVIRT_LOADER))
+GEN_NODES_EXTRA_ARGS += libvirt_loader='$(subst ",,$(CONFIG_LIBVIRT_LOADER))'
+GEN_NODES_EXTRA_ARGS += libvirt_nvram_dir='$(subst ",,$(CONFIG_LIBVIRT_NVRAM_DIR))'
+endif
+
ANSIBLE_EXTRA_ARGS += $(GEN_NODES_EXTRA_ARGS)
diff --git a/vagrant/Kconfig b/vagrant/Kconfig
index fc29f501..a8cb8895 100644
--- a/vagrant/Kconfig
+++ b/vagrant/Kconfig
@@ -12,6 +12,7 @@ config VAGRANT_LIBVIRT
config VAGRANT_VIRTUALBOX
bool "Virtualbox"
+ depends on TARGET_ARCH_X86_64
select EXTRA_STORAGE_SUPPORTS_512
select EXTRA_STORAGE_SUPPORTS_1K
select EXTRA_STORAGE_SUPPORTS_2K
@@ -198,7 +199,9 @@ endif # HAVE_LIBVIRT_PCIE_PASSTHROUGH
choice
prompt "Machine type to use"
- default LIBVIRT_MACHINE_TYPE_Q35
+ default LIBVIRT_MACHINE_TYPE_Q35 if TARGET_ARCH_X86_64
+ default LIBVIRT_MACHINE_TYPE_VIRT if TARGET_ARCH_ARM64
+ default LIBVIRT_MACHINE_TYPE_DEFAULT if (!TARGET_ARCH_X86_64 && !TARGET_ARCH_ARM64)
config LIBVIRT_MACHINE_TYPE_DEFAULT
bool "Use the default machine type"
@@ -215,10 +218,18 @@ config LIBVIRT_MACHINE_TYPE_DEFAULT
config LIBVIRT_MACHINE_TYPE_Q35
bool "q35"
+ depends on TARGET_ARCH_X86_64
help
Use q35 for the machine type. This will be required for things like
CXL or PCIe passthrough.
+config LIBVIRT_MACHINE_TYPE_VIRT
+ bool "virt"
+ depends on TARGET_ARCH_ARM64
+ select LIBVIRT_HOST_PASSTHROUGH
+ help
+ Use the arm64 virt machine type.
+
endchoice
config LIBVIRT_HOST_PASSTHROUGH
@@ -241,6 +252,64 @@ config LIBVIRT_HOST_PASSTHROUGH
--pre 'make -s mrproper defconfig' \
\-- make -s -j$(nproc) bzImage
+choice
+ prompt "Libvirt input config"
+ default LIBVIRT_INPUT_PS2_MOUSE if TARGET_ARCH_X86_64
+ default LIBVIRT_INPUT_VIRTIO_MOUSE if !TARGET_ARCH_X86_64
+
+config LIBVIRT_INPUT_PS2_MOUSE
+ bool "PS/2 mouse"
+ depends on TARGET_ARCH_X86_64
+ help
+ Use an emulated PS/2 bouse for input. Default for X86_64, not
+ supported on ARM64.
+
+config LIBVIRT_INPUT_VIRTIO_MOUSE
+ bool "Virtio mouse"
+ help
+ Use a virtio mouse for input. Default for ARM64.
+
+endchoice
+
+config LIBVIRT_FEATURE_ARM64_GIC_VERSION
+ string "GIC version"
+ depends on TARGET_ARCH_ARM64
+ default "3"
+ help
+ Specify which Generic Interrupt Controller (GIC) version libvirt
+ should use. Newer GIC versions can support more cores. For details
+ see: https://developer.arm.com/Architectures/Generic%20Interrupt%20Controller
+ Since ARM64 uses host cpu passthrough, some configs might not be
+ available due to the underlying hardware.
+
+config LIBVIRT_GRAPHICS_TYPE
+ string "Graphics type"
+ default "none" if TARGET_ARCH_ARM64
+ default "vnc" if !TARGET_ARCH_ARM64
+ help
+ Sets the protocol used to expose the guest display. Possible values
+ are `sdl`, `curses`, `none`, `gtk`, `vnc` or `spice`.
+
+config LIBVIRT_LOADER
+ string "Path to a custom UEFI loader."
+ default "/usr/share/AAVMF/AAVMF_CODE.fd" if TARGET_ARCH_ARM64
+ default "" if !TARGET_ARCH_ARM64
+ help
+ On some architectures, qemu may not automatically load the UEFI
+ firmware from the correct location. Set this variable to help it out.
+
+ You may need to install the OVMF packages for your architecture. On
+ Debian ARM64, that's qemu-efi-aarch64.
+
+config LIBVIRT_NVRAM_DIR
+ string "Path to storage for UEFI NVRAM."
+ default "/var/lib/libvirt/qemu/nvram" if TARGET_ARCH_ARM64
+ default "" if !TARGET_ARCH_ARM64
+ help
+ Some architectures may require storage on the host for UEFI NVRAM.
+ This will take 64MiB per guest. The default directory is
+ /var/lib/libvirt/qemu/nvram/.
+
config QEMU_BUILD
bool "Should we build QEMU for you?"
select NEEDS_LOCAL_DEVELOPMENT_PATH
@@ -323,8 +392,10 @@ endif # QEMU_BUILD
config QEMU_BIN_PATH_LIBVIRT
string "QEMU binary path to use"
- default "/usr/local/bin/qemu-system-x86_64" if QEMU_USE_DEVELOPMENT_VERSION
- default "/usr/bin/qemu-system-x86_64" if !QEMU_USE_DEVELOPMENT_VERSION
+ default "/usr/local/bin/qemu-system-x86_64" if TARGET_ARCH_X86_64 && QEMU_USE_DEVELOPMENT_VERSION
+ default "/usr/bin/qemu-system-x86_64" if TARGET_ARCH_X86_64 && !QEMU_USE_DEVELOPMENT_VERSION
+ default "/usr/local/bin/qemu-system-aarch64" if TARGET_ARCH_ARM64 && QEMU_USE_DEVELOPMENT_VERSION
+ default "/usr/bin/qemu-system-aarch64" if TARGET_ARCH_ARM64 && !QEMU_USE_DEVELOPMENT_VERSION
config QEMU_INSTALL_DIR_LIBVIRT
string "Path to install QEMU"
--
2.39.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] add kconfig for opensuse arm64 vagrant images
2023-09-01 5:03 [PATCH 1/2] add kconfig/ansible options for arm64 support Ross Vandegrift
@ 2023-09-01 5:03 ` Ross Vandegrift
2023-09-01 19:57 ` [PATCH 1/2] add kconfig/ansible options for arm64 support Luis Chamberlain
1 sibling, 0 replies; 5+ messages in thread
From: Ross Vandegrift @ 2023-09-01 5:03 UTC (permalink / raw)
To: mcgrof; +Cc: kdevops, Ross Vandegrift
Tumbleweed has arm64 vagrant images already and the one I tested booted up as
expected.
Signed-off-by: Ross Vandegrift <ross@kallisti.us>
---
vagrant/Kconfig.opensuse | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/vagrant/Kconfig.opensuse b/vagrant/Kconfig.opensuse
index 2ffb1f31..685018b7 100644
--- a/vagrant/Kconfig.opensuse
+++ b/vagrant/Kconfig.opensuse
@@ -3,7 +3,8 @@ if VAGRANT_OPENSUSE
choice
prompt "OpenSUSE release to use"
- default VAGRANT_OPENSUSE_X86_64_LEAP_15_3
+ default VAGRANT_OPENSUSE_X86_64_LEAP_15_3 if TARGET_ARCH_X86_64
+ default VAGRANT_OPENSUSE_ARM64_TW_20230828 if TARGET_ARCH_ARM64
config VAGRANT_OPENSUSE_X86_64_TW
bool "opensuse/Tumbleweed.x86_64"
@@ -78,6 +79,13 @@ config VAGRANT_OPENSUSE_X86_64_LEAP_15_4
help
This enables the OpenSUSE Leap 15.4 release.
+config VAGRANT_OPENSUSE_ARM64_TW
+ bool "opensuse/Tumbleweed.aarch64"
+ depends on TARGET_ARCH_ARM64
+ help
+ Enable this to be able to select between the supported OpenSUSE
+ Tumbleweed releases.
+
endchoice
if VAGRANT_OPENSUSE_X86_64_TW
@@ -157,6 +165,27 @@ endchoice
endif # VAGRANT_OPENSUSE_X86_64_TW
+if VAGRANT_OPENSUSE_ARM64_TW
+
+choice
+ prompt "opensuse/Tumbleweed.aarch64 version to use"
+ default VAGRANT_OPENSUSE_X86_64_TW_1020230828
+
+config VAGRANT_OPENSUSE_X86_64_TW_1020230828
+ bool "opensuse/Tumbleweed.aarch64 1.0.20230828"
+ select HAVE_VAGRANT_BOX_VERSION
+ select HAVE_DISTRO_XFS_SUPPORTS_CRC if FSTESTS_XFS
+ select HAVE_DISTRO_XFS_SUPPORTS_EXTERNAL_LOG if FSTESTS_XFS
+ select HAVE_DISTRO_XFS_SUPPORTS_REFLINKS if FSTESTS_XFS
+ select HAVE_DISTRO_XFS_SUPPORTS_BIGBLOCKS if FSTESTS_XFS
+ help
+ This option will set the target guest to opensuse/Tumbleweed.aarch64
+ release 1.0.20230828.
+
+endchoice
+
+endif # VAGRANT_OPENSUSE_ARM64_TW
+
config VAGRANT_OPENSUSE_BOX_SHORT
string
default "tw" if VAGRANT_OPENSUSE_X86_64_TW
--
2.39.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] add kconfig/ansible options for arm64 support
2023-09-01 5:03 [PATCH 1/2] add kconfig/ansible options for arm64 support Ross Vandegrift
2023-09-01 5:03 ` [PATCH 2/2] add kconfig for opensuse arm64 vagrant images Ross Vandegrift
@ 2023-09-01 19:57 ` Luis Chamberlain
2023-09-04 4:57 ` Ross Vandegrift
1 sibling, 1 reply; 5+ messages in thread
From: Luis Chamberlain @ 2023-09-01 19:57 UTC (permalink / raw)
To: Ross Vandegrift; +Cc: kdevops
On Thu, Aug 31, 2023 at 10:03:54PM -0700, Ross Vandegrift wrote:
> This adds new Kconfig options to support arm64 kdevops guests under vagrant +
> libvirt. They are plumbed through to ansible, and make bringup works. A few
> x86 specific things have had the dependency made explicit. I'm not sure I
> found all of those things.
>
> To test with this change, you need a local arm64 vagrant image. Debian's cloud
> images support building them, but they aren't published yet. openSUSE defaults
> will be added next.
First, thanks for doing this work!! Review below!
> diff --git a/playbooks/roles/gen_nodes/defaults/main.yml b/playbooks/roles/gen_nodes/defaults/main.yml
> index c57effe7..2cca984a 100644
> --- a/playbooks/roles/gen_nodes/defaults/main.yml
> +++ b/playbooks/roles/gen_nodes/defaults/main.yml
> @@ -60,6 +60,11 @@ libvirt_extra_storage_aio_cache_mode: "none"
> libvirt_extra_storage_nvme_logical_block_size: 512
> libvirt_extra_storage_virtio_logical_block_size: 512
> libvirt_extra_storage_virtio_physical_block_size: 512
> +libvirt_input_mouse_bus: ps2
> +libvirt_arm64_gic_version: "3"
The libvirt_arm64_gic_version seems to be an arm thing.
> +libvirt_graphics_type: vnc
> +libvirt_loader: ""
> +libvirt_nvram_dir: /var/lib/libvirt/qemu/nvram
this too.
>
> libvirt_largeio_enable: False
> libvirt_largeio_logical_compat: False
> diff --git a/playbooks/roles/gen_nodes/templates/Vagrantfile.j2 b/playbooks/roles/gen_nodes/templates/Vagrantfile.j2
> index 7ed59ff7..e064377b 100644
> --- a/playbooks/roles/gen_nodes/templates/Vagrantfile.j2
> +++ b/playbooks/roles/gen_nodes/templates/Vagrantfile.j2
> @@ -229,6 +229,21 @@ Vagrant.configure("2") do |config|
> libvirt.machine_type = global_data['libvirt_cfg']['machine_type']
> end
> end
> +
> +{% if libvirt_arm64_gic_version %}
> + libvirt.features = ["acpi", "gic version='%s'" % [global_data["libvirt_cfg"]['arm64_gic_version']]]
> +{% endif %}
The global_data is a set from the yaml file which we read:
playbooks/roles/gen_nodes/templates/Vagrantfile.j2 has:
```
config_data = YAML.load_file(node_config)
...
global_data = config_data['vagrant_global']
```
The node_config defaults to kdevops_nodes.yaml. But this was prior to us
having support and using jinja2 template engine for generating the
Vagrantfile. So now some knobs could / should be converted over to
kconfig and then we just set a default in the ansible role and we can
override the defaults.
The kdevops_nodes.yaml is also generated using a template it is split in
two files, the top level:
playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml
And then at the bottom it outputs the hosts:
playbooks/roles/gen_nodes/templates/hosts.j2
This is now documented here:
https://github.com/linux-kdevops/kdevops/blob/master/docs/the-gen-nodes-ansible-role-vagrant.md
You *are* modifying the kdevops_nodes.yaml but it is not needed if we
are just dealing with a few deafult variables that change depending on
architecure.
You can add then a bool like LIBVIRT_ARM64_ENABLE and a respective
default for libvirt_arm64_enable: False and then the Makefile add it
to True if set. In the future we will have kconfig output
extra_vars.yaml for us so we don't have to do these manual edits of the
Makefile so having name parity on the variables helps to align these
efforts. So that once we do get support for this we just remove the
Makefile changes which change a default to another value.
Then your code becomes:
{% if libvirt_arm64_enable %}
libvirt.features = ["acpi", "gic version="{{ libvirt_arm64_gic_version }}"
{% endif %}
That lets you keep arm64 stuff in the above branch.
> +{% if libvirt_graphics_type %}
> + libvirt.graphics_type = global_data["libvirt_cfg"]["graphics_type"]
> +{% endif %}
Likewise if you are always populating now libvirt_graphics_type in
Kconfig might as well always force it:
libvirt.graphics_type = "{{ libvirt_graphics_type }}"
Not sure if double quotes are needed. BTW enabling graphics type of
"none" also for x86_64 is welcomed. So a preliminary patch could be be
to add that, and default us to "none" moving forward.
Your addition of arm64 can add the depend on !TARGET_ARCH_ARM64 on "vnc".
> +{% if libvirt_input_mouse_bus %}
> + libvirt.input :type => "mouse", :bus => global_data["libvirt_cfg"]["input_mouse_bus"]
> +{% endif %}
This is generic and so if you are defining things as default no need to
use global_data thing and just use libvirt_input_mouse_bus.
> +{% if libvirt_loader %}
> + libvirt.loader = global_data["libvirt_cfg"]["loader"]
> + libvirt.nvram = "%s/%s.fd" % [global_data["libvirt_cfg"]["nvram_dir"], host_name]
> +{% endif %}
Since this seems arm64 specific this could be put under the libvirt_arm64_enable
I mentioned, and sensible default can be used. It is up to you if you
want to add the variability to this. It is more work but you already did
it so might as well have it.
> +
> # Add an extra spare PCI or PCI-E root bus to be used for extra drives.
> #
> # We use a dedicated PCI or PCI-E root bus to not clash with defaults
> diff --git a/playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml b/playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml
> index 7cd95376..367990dc 100644
> --- a/playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml
> +++ b/playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml
> @@ -24,6 +24,19 @@ vagrant_global:
> libvirt_cfg:
> {% if libvirt_override_machine_type %}
> machine_type: '{{ libvirt_machine_type }}'
> +{% endif %}
> +{% if libvirt_arm64_gic_version %}
> + arm64_gic_version: {{ libvirt_arm64_gic_version }}
> +{% endif %}
> +{% if libvirt_graphics_type %}
> + graphics_type: {{ libvirt_graphics_type }}
> +{% endif %}
> +{% if libvirt_input_mouse_bus %}
> + input_mouse_bus: {{ libvirt_input_mouse_bus }}
> +{% endif %}
> +{% if libvirt_loader %}
> + loader: {{ libvirt_loader }}
> + nvram_dir: {{ libvirt_nvram_dir }}
> {% endif %}
> # This lets the Ansible role kdevops_vagrant try to infer your default
> # distro group to use for QEMU. OpenSUSE and Fedora uses QEMU here by
So this becomes not needed.
> diff --git a/scripts/gen-nodes.Makefile b/scripts/gen-nodes.Makefile
> index 533b19db..5be1d1ba 100644
> --- a/scripts/gen-nodes.Makefile
> +++ b/scripts/gen-nodes.Makefile
> @@ -203,4 +203,28 @@ endif # CONFIG_QEMU_ENABLE_CXL
>
> endif # CONFIG_LIBVIRT_MACHINE_TYPE_Q35
>
> +ifeq (y,$(CONFIG_LIBVIRT_MACHINE_TYPE_VIRT))
> +GEN_NODES_EXTRA_ARGS += libvirt_override_machine_type='True'
> +GEN_NODES_EXTRA_ARGS += libvirt_machine_type='virt'
> +endif
> +
> +ifeq (y,$(CONFIG_LIBVIRT_INPUT_PS2_MOUSE))
> +GEN_NODES_EXTRA_ARGS += libvirt_input_mouse_bus='ps2'
> +else ifeq (y, $(CONFIG_LIBVIRT_INPUT_VIRTIO_MOUSE))
> +GEN_NODES_EXTRA_ARGS += libvirt_input_mouse_bus='virtio'
> +endif
> +
> +ifneq (,$(CONFIG_LIBVIRT_FEATURE_ARM64_GIC_VERSION))
> +GEN_NODES_EXTRA_ARGS += libvirt_arm64_gic_version='$(subst ",,$(CONFIG_LIBVIRT_FEATURE_ARM64_GIC_VERSION))'
> +endif
> +
> +ifneq (,$(CONFIG_LIBVIRT_GRAPHICS_TYPE))
> +GEN_NODES_EXTRA_ARGS += libvirt_graphics_type='$(subst ",,$(CONFIG_LIBCIRT_GRAPHICS_TYPE))'
> +endif
> +
> +ifneq (,$(CONFIG_LIBVIRT_LOADER))
> +GEN_NODES_EXTRA_ARGS += libvirt_loader='$(subst ",,$(CONFIG_LIBVIRT_LOADER))'
> +GEN_NODES_EXTRA_ARGS += libvirt_nvram_dir='$(subst ",,$(CONFIG_LIBVIRT_NVRAM_DIR))'
> +endif
> +
> ANSIBLE_EXTRA_ARGS += $(GEN_NODES_EXTRA_ARGS)
> diff --git a/vagrant/Kconfig b/vagrant/Kconfig
> index fc29f501..a8cb8895 100644
> --- a/vagrant/Kconfig
> +++ b/vagrant/Kconfig
> @@ -12,6 +12,7 @@ config VAGRANT_LIBVIRT
>
> config VAGRANT_VIRTUALBOX
> bool "Virtualbox"
> + depends on TARGET_ARCH_X86_64
> select EXTRA_STORAGE_SUPPORTS_512
> select EXTRA_STORAGE_SUPPORTS_1K
> select EXTRA_STORAGE_SUPPORTS_2K
> @@ -198,7 +199,9 @@ endif # HAVE_LIBVIRT_PCIE_PASSTHROUGH
>
> choice
> prompt "Machine type to use"
> - default LIBVIRT_MACHINE_TYPE_Q35
> + default LIBVIRT_MACHINE_TYPE_Q35 if TARGET_ARCH_X86_64
> + default LIBVIRT_MACHINE_TYPE_VIRT if TARGET_ARCH_ARM64
> + default LIBVIRT_MACHINE_TYPE_DEFAULT if (!TARGET_ARCH_X86_64 && !TARGET_ARCH_ARM64)
>
> config LIBVIRT_MACHINE_TYPE_DEFAULT
> bool "Use the default machine type"
> @@ -215,10 +218,18 @@ config LIBVIRT_MACHINE_TYPE_DEFAULT
>
> config LIBVIRT_MACHINE_TYPE_Q35
> bool "q35"
> + depends on TARGET_ARCH_X86_64
> help
> Use q35 for the machine type. This will be required for things like
> CXL or PCIe passthrough.
>
> +config LIBVIRT_MACHINE_TYPE_VIRT
> + bool "virt"
> + depends on TARGET_ARCH_ARM64
> + select LIBVIRT_HOST_PASSTHROUGH
> + help
> + Use the arm64 virt machine type.
> +
> endchoice
>
> config LIBVIRT_HOST_PASSTHROUGH
> @@ -241,6 +252,64 @@ config LIBVIRT_HOST_PASSTHROUGH
> --pre 'make -s mrproper defconfig' \
> \-- make -s -j$(nproc) bzImage
>
> +choice
> + prompt "Libvirt input config"
> + default LIBVIRT_INPUT_PS2_MOUSE if TARGET_ARCH_X86_64
> + default LIBVIRT_INPUT_VIRTIO_MOUSE if !TARGET_ARCH_X86_64
> +
> +config LIBVIRT_INPUT_PS2_MOUSE
> + bool "PS/2 mouse"
> + depends on TARGET_ARCH_X86_64
> + help
> + Use an emulated PS/2 bouse for input. Default for X86_64, not
> + supported on ARM64.
> +
> +config LIBVIRT_INPUT_VIRTIO_MOUSE
> + bool "Virtio mouse"
> + help
> + Use a virtio mouse for input. Default for ARM64.
> +
> +endchoice
You can make the choise have a follow up Kconfig string without a label,
using the same name of the default you are using on the ansible role.
The name parity is so that when we extend kconfig to support outputting
extra_vars.yaml we can remove the Makeifle edits as they would no longer
be needed. This let's us scale faster.
For example, see commits:
95f5695f9038 ("fstests: remove redundant string prompts for fstype")
9bd64d280e31 ("data partition: remove string on kconfig WORKFLOW_DATA_FSTYPE")
40be7e188f43 ("vagrant: remove redundant string for cpu and mem choice")
These are examples of how using a Kconfig string is used to let us
populate variables easily.
Otherwise, looks good!
Luis
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] add kconfig/ansible options for arm64 support
2023-09-01 19:57 ` [PATCH 1/2] add kconfig/ansible options for arm64 support Luis Chamberlain
@ 2023-09-04 4:57 ` Ross Vandegrift
2023-09-05 20:04 ` Luis Chamberlain
0 siblings, 1 reply; 5+ messages in thread
From: Ross Vandegrift @ 2023-09-04 4:57 UTC (permalink / raw)
To: Luis Chamberlain; +Cc: kdevops
Hi Luis,
Thanks for the review. I have one further question about the arm64-specific
approach below.
On Fri, Sep 01, 2023 at 12:57:21PM -0700, Luis Chamberlain wrote:
> On Thu, Aug 31, 2023 at 10:03:54PM -0700, Ross Vandegrift wrote:
> > This adds new Kconfig options to support arm64 kdevops guests under vagrant +
> > libvirt. They are plumbed through to ansible, and make bringup works. A few
> > x86 specific things have had the dependency made explicit. I'm not sure I
> > found all of those things.
> >
> > To test with this change, you need a local arm64 vagrant image. Debian's cloud
> > images support building them, but they aren't published yet. openSUSE defaults
> > will be added next.
>
> First, thanks for doing this work!! Review below!
>
> > diff --git a/playbooks/roles/gen_nodes/defaults/main.yml b/playbooks/roles/gen_nodes/defaults/main.yml
> > index c57effe7..2cca984a 100644
> > --- a/playbooks/roles/gen_nodes/defaults/main.yml
> > +++ b/playbooks/roles/gen_nodes/defaults/main.yml
> > @@ -60,6 +60,11 @@ libvirt_extra_storage_aio_cache_mode: "none"
> > libvirt_extra_storage_nvme_logical_block_size: 512
> > libvirt_extra_storage_virtio_logical_block_size: 512
> > libvirt_extra_storage_virtio_physical_block_size: 512
> > +libvirt_input_mouse_bus: ps2
> > +libvirt_arm64_gic_version: "3"
>
> The libvirt_arm64_gic_version seems to be an arm thing.
That's right.
> > +libvirt_graphics_type: vnc
> > +libvirt_loader: ""
> > +libvirt_nvram_dir: /var/lib/libvirt/qemu/nvram
>
> this too.
I'm not sure about this one. `loader` works on any platform afaik. On amd64,
qemu's default searches and finds the firmware. On arm64, it doesn't search,
or the search fails (not sure which). But the parameter is available on either
arch.
`nvram` is similar. On arm64, I can't boot without specifying a file. On
amd64, guests work without it. But it is allowed - compare `QEMU with UEFI
manual firmware and secure boot` and `QEMU with UEFI manual firmware, secure
boot and with NVRAM type 'file'` at:
https://libvirt.org/formatdomain.html#bios-bootloader
This made me think there could be cases where and amd64 user might need to
override these. But this is outside of my wheelhouse, so I don't know any real
use-cases. If you'd prefer them only available on arm64 I'm happy to make that
change.
> > libvirt_largeio_enable: False
> > libvirt_largeio_logical_compat: False
> > diff --git a/playbooks/roles/gen_nodes/templates/Vagrantfile.j2 b/playbooks/roles/gen_nodes/templates/Vagrantfile.j2
> > index 7ed59ff7..e064377b 100644
> > --- a/playbooks/roles/gen_nodes/templates/Vagrantfile.j2
> > +++ b/playbooks/roles/gen_nodes/templates/Vagrantfile.j2
> > @@ -229,6 +229,21 @@ Vagrant.configure("2") do |config|
> > libvirt.machine_type = global_data['libvirt_cfg']['machine_type']
> > end
> > end
> > +
> > +{% if libvirt_arm64_gic_version %}
> > + libvirt.features = ["acpi", "gic version='%s'" % [global_data["libvirt_cfg"]['arm64_gic_version']]]
> > +{% endif %}
>
> The global_data is a set from the yaml file which we read:
>
> playbooks/roles/gen_nodes/templates/Vagrantfile.j2 has:
>
> ```
> config_data = YAML.load_file(node_config)
> ...
> global_data = config_data['vagrant_global']
> ```
>
> The node_config defaults to kdevops_nodes.yaml. But this was prior to us
> having support and using jinja2 template engine for generating the
> Vagrantfile. So now some knobs could / should be converted over to
> kconfig and then we just set a default in the ansible role and we can
> override the defaults.
>
> The kdevops_nodes.yaml is also generated using a template it is split in
> two files, the top level:
>
> playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml
>
> And then at the bottom it outputs the hosts:
>
> playbooks/roles/gen_nodes/templates/hosts.j2
>
> This is now documented here:
>
> https://github.com/linux-kdevops/kdevops/blob/master/docs/the-gen-nodes-ansible-role-vagrant.md
>
> You *are* modifying the kdevops_nodes.yaml but it is not needed if we
> are just dealing with a few deafult variables that change depending on
> architecure.
>
> You can add then a bool like LIBVIRT_ARM64_ENABLE and a respective
> default for libvirt_arm64_enable: False and then the Makefile add it
> to True if set. In the future we will have kconfig output
> extra_vars.yaml for us so we don't have to do these manual edits of the
> Makefile so having name parity on the variables helps to align these
> efforts. So that once we do get support for this we just remove the
> Makefile changes which change a default to another value.
> Then your code becomes:
>
> {% if libvirt_arm64_enable %}
> libvirt.features = ["acpi", "gic version="{{ libvirt_arm64_gic_version }}"
> {% endif %}
>
> That lets you keep arm64 stuff in the above branch.
>
> > +{% if libvirt_graphics_type %}
> > + libvirt.graphics_type = global_data["libvirt_cfg"]["graphics_type"]
> > +{% endif %}
>
> Likewise if you are always populating now libvirt_graphics_type in
> Kconfig might as well always force it:
>
> libvirt.graphics_type = "{{ libvirt_graphics_type }}"
>
> Not sure if double quotes are needed. BTW enabling graphics type of
> "none" also for x86_64 is welcomed. So a preliminary patch could be be
> to add that, and default us to "none" moving forward.
>
> Your addition of arm64 can add the depend on !TARGET_ARCH_ARM64 on "vnc".
>
> > +{% if libvirt_input_mouse_bus %}
> > + libvirt.input :type => "mouse", :bus => global_data["libvirt_cfg"]["input_mouse_bus"]
> > +{% endif %}
>
> This is generic and so if you are defining things as default no need to
> use global_data thing and just use libvirt_input_mouse_bus.
Ah ansible variables are implicitly made available to jinja! Somehow I missed
that. Great, much better.
> > +{% if libvirt_loader %}
> > + libvirt.loader = global_data["libvirt_cfg"]["loader"]
> > + libvirt.nvram = "%s/%s.fd" % [global_data["libvirt_cfg"]["nvram_dir"], host_name]
> > +{% endif %}
>
> Since this seems arm64 specific this could be put under the libvirt_arm64_enable
> I mentioned, and sensible default can be used. It is up to you if you
> want to add the variability to this. It is more work but you already did
> it so might as well have it.
>
> > +
> > # Add an extra spare PCI or PCI-E root bus to be used for extra drives.
> > #
> > # We use a dedicated PCI or PCI-E root bus to not clash with defaults
> > diff --git a/playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml b/playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml
> > index 7cd95376..367990dc 100644
> > --- a/playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml
> > +++ b/playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml
> > @@ -24,6 +24,19 @@ vagrant_global:
> > libvirt_cfg:
> > {% if libvirt_override_machine_type %}
> > machine_type: '{{ libvirt_machine_type }}'
> > +{% endif %}
> > +{% if libvirt_arm64_gic_version %}
> > + arm64_gic_version: {{ libvirt_arm64_gic_version }}
> > +{% endif %}
> > +{% if libvirt_graphics_type %}
> > + graphics_type: {{ libvirt_graphics_type }}
> > +{% endif %}
> > +{% if libvirt_input_mouse_bus %}
> > + input_mouse_bus: {{ libvirt_input_mouse_bus }}
> > +{% endif %}
> > +{% if libvirt_loader %}
> > + loader: {{ libvirt_loader }}
> > + nvram_dir: {{ libvirt_nvram_dir }}
> > {% endif %}
> > # This lets the Ansible role kdevops_vagrant try to infer your default
> > # distro group to use for QEMU. OpenSUSE and Fedora uses QEMU here by
>
> So this becomes not needed.
🤘
> > diff --git a/scripts/gen-nodes.Makefile b/scripts/gen-nodes.Makefile
> > index 533b19db..5be1d1ba 100644
> > --- a/scripts/gen-nodes.Makefile
> > +++ b/scripts/gen-nodes.Makefile
> > @@ -203,4 +203,28 @@ endif # CONFIG_QEMU_ENABLE_CXL
> >
> > endif # CONFIG_LIBVIRT_MACHINE_TYPE_Q35
> >
> > +ifeq (y,$(CONFIG_LIBVIRT_MACHINE_TYPE_VIRT))
> > +GEN_NODES_EXTRA_ARGS += libvirt_override_machine_type='True'
> > +GEN_NODES_EXTRA_ARGS += libvirt_machine_type='virt'
> > +endif
> > +
> > +ifeq (y,$(CONFIG_LIBVIRT_INPUT_PS2_MOUSE))
> > +GEN_NODES_EXTRA_ARGS += libvirt_input_mouse_bus='ps2'
> > +else ifeq (y, $(CONFIG_LIBVIRT_INPUT_VIRTIO_MOUSE))
> > +GEN_NODES_EXTRA_ARGS += libvirt_input_mouse_bus='virtio'
> > +endif
> > +
> > +ifneq (,$(CONFIG_LIBVIRT_FEATURE_ARM64_GIC_VERSION))
> > +GEN_NODES_EXTRA_ARGS += libvirt_arm64_gic_version='$(subst ",,$(CONFIG_LIBVIRT_FEATURE_ARM64_GIC_VERSION))'
> > +endif
> > +
> > +ifneq (,$(CONFIG_LIBVIRT_GRAPHICS_TYPE))
> > +GEN_NODES_EXTRA_ARGS += libvirt_graphics_type='$(subst ",,$(CONFIG_LIBCIRT_GRAPHICS_TYPE))'
> > +endif
> > +
> > +ifneq (,$(CONFIG_LIBVIRT_LOADER))
> > +GEN_NODES_EXTRA_ARGS += libvirt_loader='$(subst ",,$(CONFIG_LIBVIRT_LOADER))'
> > +GEN_NODES_EXTRA_ARGS += libvirt_nvram_dir='$(subst ",,$(CONFIG_LIBVIRT_NVRAM_DIR))'
> > +endif
> > +
> > ANSIBLE_EXTRA_ARGS += $(GEN_NODES_EXTRA_ARGS)
> > diff --git a/vagrant/Kconfig b/vagrant/Kconfig
> > index fc29f501..a8cb8895 100644
> > --- a/vagrant/Kconfig
> > +++ b/vagrant/Kconfig
> > @@ -12,6 +12,7 @@ config VAGRANT_LIBVIRT
> >
> > config VAGRANT_VIRTUALBOX
> > bool "Virtualbox"
> > + depends on TARGET_ARCH_X86_64
> > select EXTRA_STORAGE_SUPPORTS_512
> > select EXTRA_STORAGE_SUPPORTS_1K
> > select EXTRA_STORAGE_SUPPORTS_2K
> > @@ -198,7 +199,9 @@ endif # HAVE_LIBVIRT_PCIE_PASSTHROUGH
> >
> > choice
> > prompt "Machine type to use"
> > - default LIBVIRT_MACHINE_TYPE_Q35
> > + default LIBVIRT_MACHINE_TYPE_Q35 if TARGET_ARCH_X86_64
> > + default LIBVIRT_MACHINE_TYPE_VIRT if TARGET_ARCH_ARM64
> > + default LIBVIRT_MACHINE_TYPE_DEFAULT if (!TARGET_ARCH_X86_64 && !TARGET_ARCH_ARM64)
> >
> > config LIBVIRT_MACHINE_TYPE_DEFAULT
> > bool "Use the default machine type"
> > @@ -215,10 +218,18 @@ config LIBVIRT_MACHINE_TYPE_DEFAULT
> >
> > config LIBVIRT_MACHINE_TYPE_Q35
> > bool "q35"
> > + depends on TARGET_ARCH_X86_64
> > help
> > Use q35 for the machine type. This will be required for things like
> > CXL or PCIe passthrough.
> >
> > +config LIBVIRT_MACHINE_TYPE_VIRT
> > + bool "virt"
> > + depends on TARGET_ARCH_ARM64
> > + select LIBVIRT_HOST_PASSTHROUGH
> > + help
> > + Use the arm64 virt machine type.
> > +
> > endchoice
> >
> > config LIBVIRT_HOST_PASSTHROUGH
> > @@ -241,6 +252,64 @@ config LIBVIRT_HOST_PASSTHROUGH
> > --pre 'make -s mrproper defconfig' \
> > \-- make -s -j$(nproc) bzImage
> >
> > +choice
> > + prompt "Libvirt input config"
> > + default LIBVIRT_INPUT_PS2_MOUSE if TARGET_ARCH_X86_64
> > + default LIBVIRT_INPUT_VIRTIO_MOUSE if !TARGET_ARCH_X86_64
> > +
> > +config LIBVIRT_INPUT_PS2_MOUSE
> > + bool "PS/2 mouse"
> > + depends on TARGET_ARCH_X86_64
> > + help
> > + Use an emulated PS/2 bouse for input. Default for X86_64, not
> > + supported on ARM64.
> > +
> > +config LIBVIRT_INPUT_VIRTIO_MOUSE
> > + bool "Virtio mouse"
> > + help
> > + Use a virtio mouse for input. Default for ARM64.
> > +
> > +endchoice
>
> You can make the choise have a follow up Kconfig string without a label,
> using the same name of the default you are using on the ansible role.
> The name parity is so that when we extend kconfig to support outputting
> extra_vars.yaml we can remove the Makeifle edits as they would no longer
> be needed. This let's us scale faster.
>
> For example, see commits:
>
> 95f5695f9038 ("fstests: remove redundant string prompts for fstype")
> 9bd64d280e31 ("data partition: remove string on kconfig WORKFLOW_DATA_FSTYPE")
> 40be7e188f43 ("vagrant: remove redundant string for cpu and mem choice")
>
> These are examples of how using a Kconfig string is used to let us
> populate variables easily.
Interesting, thanks for the tip!
Ross
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] add kconfig/ansible options for arm64 support
2023-09-04 4:57 ` Ross Vandegrift
@ 2023-09-05 20:04 ` Luis Chamberlain
0 siblings, 0 replies; 5+ messages in thread
From: Luis Chamberlain @ 2023-09-05 20:04 UTC (permalink / raw)
To: Ross Vandegrift; +Cc: kdevops
On Sun, Sep 03, 2023 at 09:57:40PM -0700, Ross Vandegrift wrote:
> On Fri, Sep 01, 2023 at 12:57:21PM -0700, Luis Chamberlain wrote:
> > On Thu, Aug 31, 2023 at 10:03:54PM -0700, Ross Vandegrift wrote:
> > > +libvirt_graphics_type: vnc
> > > +libvirt_loader: ""
> > > +libvirt_nvram_dir: /var/lib/libvirt/qemu/nvram
> >
> > this too.
>
> I'm not sure about this one. `loader` works on any platform afaik. On amd64,
> qemu's default searches and finds the firmware. On arm64, it doesn't search,
> or the search fails (not sure which). But the parameter is available on either
> arch.
Ah OK!
> `nvram` is similar. On arm64, I can't boot without specifying a file. On
> amd64, guests work without it. But it is allowed - compare `QEMU with UEFI
> manual firmware and secure boot` and `QEMU with UEFI manual firmware, secure
> boot and with NVRAM type 'file'` at:
> https://libvirt.org/formatdomain.html#bios-bootloader
>
> This made me think there could be cases where and amd64 user might need to
> override these. But this is outside of my wheelhouse, so I don't know any real
> use-cases.
These little nuggets are extremely useful obscure docs which we could
add as Kconfig, but only if you are willing to go through the trouble
of adding a Kconfig entry for this just for that. Otherwise a simple
default to empty with an override for ARM is fine for now.
> If you'd prefer them only available on arm64 I'm happy to make that
> change.
The minimum work then is a default empty with an override for arm64.
If you want to do more work which may be helpful for others to document
the use case in Kconfig why this is also available for non-arm64. Up to
you!
Luis
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-09-05 20:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-01 5:03 [PATCH 1/2] add kconfig/ansible options for arm64 support Ross Vandegrift
2023-09-01 5:03 ` [PATCH 2/2] add kconfig for opensuse arm64 vagrant images Ross Vandegrift
2023-09-01 19:57 ` [PATCH 1/2] add kconfig/ansible options for arm64 support Luis Chamberlain
2023-09-04 4:57 ` Ross Vandegrift
2023-09-05 20:04 ` Luis Chamberlain
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.