Re: [RFC kvmtool 1/9] Drop support for 32-bit arm

[Oliver Upton <oliver.upton@linux.dev>]

On Thu, Mar 20, 2025 at 04:58:53PM +0000, Alexandru Elisei wrote:
> Hi Oliver,
> 
> I tried to apply the patch on top of e48563f5c4a48fe6a6bc2a98a9a7c84a10f043be,
> which is the base commit from the cover letter, and I got these errors:
> 
> Applying: Drop support for 32-bit arm
> error: removal patch leaves file contents
> error: arm/aarch32/arm-cpu.c: patch does not apply
> error: removal patch leaves file contents
> error: arm/aarch32/include/asm/kernel.h: patch does not apply
> error: removal patch leaves file contents
> error: arm/aarch32/include/asm/kvm.h: patch does not apply
> error: removal patch leaves file contents
> error: arm/aarch32/include/kvm/barrier.h: patch does not apply
> error: removal patch leaves file contents
> error: arm/aarch32/include/kvm/fdt-arch.h: patch does not apply
> error: removal patch leaves file contents
> error: arm/aarch32/include/kvm/kvm-arch.h: patch does not apply
> error: removal patch leaves file contents
> error: arm/aarch32/include/kvm/kvm-config-arch.h: patch does not apply
> error: removal patch leaves file contents
> error: arm/aarch32/include/kvm/kvm-cpu-arch.h: patch does not apply
> error: removal patch leaves file contents
> error: arm/aarch32/kvm-cpu.c: patch does not apply
> error: removal patch leaves file contents
> error: arm/aarch32/kvm.c: patch does not apply
> 
> When I delete the files manually, the resulting commit has diffs like this for
> the deleted files:
> 
> diff --git a/arm/aarch32/arm-cpu.c b/arm/aarch32/arm-cpu.c
> deleted file mode 100644
> index 16bba5524caf..000000000000
> --- a/arm/aarch32/arm-cpu.c
> +++ /dev/null
> @@ -1,50 +0,0 @@
> -#include "kvm/kvm.h"
> -#include "kvm/kvm-cpu.h"
> -#include "kvm/util.h"
> -
> -#include "arm-common/gic.h"
> -#include "arm-common/timer.h"
> [..]
> 
> .. and so on.
> 
> Am I missing a knob for applying the patch? FYI, this happens for all the
> patches in this series with files deleted.

That's on me, I did 'git format-patch -D' to keep the diffs smaller for
an RFC. I'll post patches that actually apply for v1.

> One more comment below.
> 
> On Fri, Mar 14, 2025 at 03:25:08PM -0700, Oliver Upton wrote:
> > Linux dropped support for KVM in 32-bit arm kernels almost 5 years ago
> > in the 5.7 kernel release. In addition to that KVM/arm64 never had
> > 32-bit compat support, so it is a safe assumption that usage of 32-bit
> > kvmtool is pretty much dead at this point.
> > 
> > Do not despair -- 32-bit guests are still supported with a 64-bit
> > userspace.
> > 
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> >  INSTALL                                   |   9 +-
> >  Makefile                                  |  31 +--
> >  arm/aarch32/arm-cpu.c                     |  50 ----
> >  arm/aarch32/include/asm/kernel.h          |   8 -
> >  arm/aarch32/include/asm/kvm.h             | 311 ----------------------
> >  arm/aarch32/include/kvm/barrier.h         |  10 -
> >  arm/aarch32/include/kvm/fdt-arch.h        |   6 -
> >  arm/aarch32/include/kvm/kvm-arch.h        |  18 --
> >  arm/aarch32/include/kvm/kvm-config-arch.h |   8 -
> >  arm/aarch32/include/kvm/kvm-cpu-arch.h    |  24 --
> >  arm/aarch32/kvm-cpu.c                     | 132 ---------
> >  arm/aarch32/kvm.c                         |  14 -
> >  12 files changed, 14 insertions(+), 607 deletions(-)
> >  delete mode 100644 arm/aarch32/arm-cpu.c
> >  delete mode 100644 arm/aarch32/include/asm/kernel.h
> >  delete mode 100644 arm/aarch32/include/asm/kvm.h
> >  delete mode 100644 arm/aarch32/include/kvm/barrier.h
> >  delete mode 100644 arm/aarch32/include/kvm/fdt-arch.h
> >  delete mode 100644 arm/aarch32/include/kvm/kvm-arch.h
> >  delete mode 100644 arm/aarch32/include/kvm/kvm-config-arch.h
> >  delete mode 100644 arm/aarch32/include/kvm/kvm-cpu-arch.h
> >  delete mode 100644 arm/aarch32/kvm-cpu.c
> >  delete mode 100644 arm/aarch32/kvm.c
> > 
> > diff --git a/INSTALL b/INSTALL
> > index 2a65735..0e1e63e 100644
> > --- a/INSTALL
> > +++ b/INSTALL
> > @@ -26,7 +26,7 @@ For Fedora based systems:
> >  For OpenSUSE based systems:
> >  	# zypper install glibc-devel-static
> >  
> > -Architectures which require device tree (PowerPC, ARM, ARM64, RISC-V) also
> > +Architectures which require device tree (PowerPC, ARM64, RISC-V) also
> >  require libfdt.
> >  	deb: $ sudo apt-get install libfdt-dev
> >  	Fedora: # yum install libfdt-devel
> > @@ -61,16 +61,15 @@ to the Linux name of the architecture. Architectures supported:
> >  - i386
> >  - x86_64
> >  - powerpc
> > -- arm
> >  - arm64
> >  - mips
> >  - riscv
> >  If ARCH is not provided, the target architecture will be automatically
> >  determined by running "uname -m" on your host, resulting in a native build.
> >  
> > -To cross-compile to ARM for instance, install a cross-compiler, put the
> > +To cross-compile to arm64 for instance, install a cross-compiler, put the
> >  required libraries in the cross-compiler's SYSROOT and type:
> > -$ make CROSS_COMPILE=arm-linux-gnueabihf- ARCH=arm
> > +$ make CROSS_COMPILE=aarch64-linux-gnu- ARCH=arm64
> >  
> >  Missing libraries when cross-compiling
> >  ---------------------------------------
> > @@ -82,7 +81,7 @@ On multiarch system you should be able to install those be appending
> >  the architecture name after the package (example for ARM64):
> >  $ sudo apt-get install libfdt-dev:arm64
> >  
> > -PowerPC, ARM/ARM64 and RISC-V require libfdt to be installed. If you cannot use
> > +PowerPC, ARM64 and RISC-V require libfdt to be installed. If you cannot use
> >  precompiled mulitarch packages, you could either copy the required header and
> >  library files from an installed target system into the SYSROOT (you will need
> >  /usr/include/*fdt*.h and /usr/lib64/libfdt-v.v.v.so and its symlinks), or you
> > diff --git a/Makefile b/Makefile
> > index d84dc8e..462659b 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -166,35 +166,24 @@ ifeq ($(ARCH), powerpc)
> >  	ARCH_WANT_LIBFDT := y
> >  endif
> >  
> > -# ARM
> > -OBJS_ARM_COMMON		:= arm/fdt.o arm/gic.o arm/gicv2m.o arm/ioport.o \
> > -			   arm/kvm.o arm/kvm-cpu.o arm/pci.o arm/timer.o \
> > -			   hw/serial.o
> > -HDRS_ARM_COMMON		:= arm/include
> > -ifeq ($(ARCH), arm)
> > -	DEFINES		+= -DCONFIG_ARM
> 
> Found a couple of instances of CONFIG_ARM using grep.
> 
> There also one instance of the architecture name ARM in a comment in
> hw/cfi_flash.c, I think that was a typo and it was meant to say ARM64.
> 
> Other than that, looks good.

Thanks for spotting those, will fix.

Thanks,
Oliver