7cd50cbe | 27-Jun-2023 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw: Don't use __bss_start with the "larl" instruction
start.S currently cannot be compiled with Clang 16 and binutils 2.40:
ld: start.o(.text+0x8): misaligned symbol `__bss_start' (0
pc-bios/s390-ccw: Don't use __bss_start with the "larl" instruction
start.S currently cannot be compiled with Clang 16 and binutils 2.40:
ld: start.o(.text+0x8): misaligned symbol `__bss_start' (0xc1e5) for relocation R_390_PC32DBL
According to the built-in linker script of ld, the symbol __bss_start can actually point *before* the .bss section and does not need to have any alignment, so in certain situations (like when using the internal assembler of Clang), the __bss_start symbol can indeed be unaligned and thus it is not suitable for being used with the "larl" instruction that needs an address that is at least aligned to halfwords. The problem went unnoticed so far since binutils <= 2.39 did not check the alignment, but starting with binutils 2.40, such unaligned addresses are now refused.
Fix it by loading the address indirectly instead.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2216662 Reported-by: Miroslav Rezanina <mrezanin@redhat.com> Suggested-by: Andreas Krebbel <andreas.krebbel@de.ibm.com> Message-Id: <20230629104821.194859-8-thuth@redhat.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|
e31f08dc | 27-Jun-2023 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw: Move the stack array into start.S
The stack array is only referenced from the start-up code (which is shared between the s390-ccw.img and the s390-netboot.img), but it is currently
pc-bios/s390-ccw: Move the stack array into start.S
The stack array is only referenced from the start-up code (which is shared between the s390-ccw.img and the s390-netboot.img), but it is currently declared twice, once in main.c and once in netmain.c. It makes more sense to declare this in start.S instead - which will also be helpful in the next patch, since we need to mention the .bss section in start.S in that patch.
While we're at it, let's also drop the huge alignment of the stack, since there is no technical requirement for aligning it to page boundaries.
Message-Id: <20230627074703.99608-4-thuth@redhat.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reviewed-by: Eric Farman <farman@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|
74fe98ee | 27-Jun-2023 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw: Provide space for initial stack frame in start.S
Providing the space of a stack frame is the duty of the caller, so we should reserve 160 bytes before jumping into the main functio
pc-bios/s390-ccw: Provide space for initial stack frame in start.S
Providing the space of a stack frame is the duty of the caller, so we should reserve 160 bytes before jumping into the main function. Otherwise the main() function might write past the stack array.
While we're at it, add a proper STACK_SIZE macro for the stack size instead of using magic numbers (this is also required for the following patch).
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Message-Id: <20230627074703.99608-3-thuth@redhat.com> Reviewed-by: Eric Farman <farman@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|
f52420fa | 27-Jun-2023 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw: Fix indentation in start.S
start.S is currently indented with a mixture of spaces and tabs, which is quite ugly. QEMU coding style says indentation should be 4 spaces, and this is
pc-bios/s390-ccw: Fix indentation in start.S
start.S is currently indented with a mixture of spaces and tabs, which is quite ugly. QEMU coding style says indentation should be 4 spaces, and this is also what we are using in the assembler files in the tests/tcg/s390x/ folder already, so let's adjust start.S accordingly.
Reviewed-by: Cédric Le Goater <clg@redhat.com> Message-Id: <20230627074703.99608-2-thuth@redhat.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reviewed-by: Eric Farman <farman@linux.ibm.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|
442ef32e | 22-Jun-2023 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw/Makefile: Use -z noexecstack to silence linker warning
Recent versions of ld complain when linking the s390-ccw bios:
/usr/bin/ld: warning: start.o: missing .note.GNU-stack sectio
pc-bios/s390-ccw/Makefile: Use -z noexecstack to silence linker warning
Recent versions of ld complain when linking the s390-ccw bios:
/usr/bin/ld: warning: start.o: missing .note.GNU-stack section implies executable stack /usr/bin/ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker
We can silence the warning by telling the linker to mark the stack as not executable.
Message-Id: <20230622130822.396793-1-thuth@redhat.com> Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|
0c2a6e12 | 27-Jun-2023 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw: Get rid of the the __u* types
The types starting with double underscores have likely been introduced into the s390-ccw bios to be able to re-use structs from the Linux kernel in th
pc-bios/s390-ccw: Get rid of the the __u* types
The types starting with double underscores have likely been introduced into the s390-ccw bios to be able to re-use structs from the Linux kernel in the past, but the corresponding structs in cio.h have been changed there a long time ago already to not use the variants with the double underscores anymore:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/diff/drivers/s390/cio/cio.h?id=cd6b4f27b9bb2a
So it would be good to replace these in the s390-ccw bios now, too.
Message-Id: <20230627114101.122231-1-thuth@redhat.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reviewed-by: Eric Farman <farman@linux.ibm.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|
bf6903f6 | 19-May-2023 |
Paolo Bonzini <pbonzini@redhat.com> |
pc-bios/s390-ccw: always build network bootloader
In the beginning, the network bootloader was considered experimental and thus optional, but it is well established nowadays and configure always che
pc-bios/s390-ccw: always build network bootloader
In the beginning, the network bootloader was considered experimental and thus optional, but it is well established nowadays and configure always checks for roms/SLOF before compiling pc-bios/s390-ccw.
Therefore, it makes sense to always build it together with the other part of the s390-ccw bios.
Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
show more ...
|
393296de | 05-Aug-2022 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw: Fix booting with logical block size < physical block size
For accessing single blocks during boot, it's the logical block size that matters. (Physical block sizes are rather intere
pc-bios/s390-ccw: Fix booting with logical block size < physical block size
For accessing single blocks during boot, it's the logical block size that matters. (Physical block sizes are rather interesting e.g. for creating file systems with the correct alignment for speed reasons etc.). So the s390-ccw bios has to use the logical block size for calculating sector numbers during the boot phase, the "physical_block_exp" shift value must not be taken into account. This change fixes the boot process when the guest hast been installed on a disk where the logical block size differs from the physical one, e.g. if the guest has been installed like this:
qemu-system-s390x -nographic -accel kvm -m 2G \ -drive if=none,id=d1,file=fedora.iso,format=raw,media=cdrom \ -device virtio-scsi -device scsi-cd,drive=d1 \ -drive if=none,id=d2,file=test.qcow2,format=qcow2 -device virtio-blk,drive=d2,physical_block_size=4096,logical_block_size=512
Linux correctly uses the logical block size of 512 for the installation, but the s390-ccw bios tries to boot from a disk with 4096 block size so far, as long as this patch has not been applied yet (well, it used to work by accident in the past due to the virtio_assume_scsi() hack that used to enforce 512 byte sectors on all virtio-block disks, but that hack has been well removed in commit 5447de2619050a0a4d to fix other scenarios).
Fixes: 5447de2619 ("pc-bios/s390-ccw/virtio-blkdev: Remove virtio_assume_scsi()") Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2112303 Message-Id: <20220805094214.285223-1-thuth@redhat.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Eric Farman <farman@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|
e2269220 | 04-Jul-2022 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw/netboot.mak: Ignore Clang's warnings about GNU extensions
When compiling the s390-ccw bios with Clang (v14.0), there is currently an unuseful warning like this:
CC pc-bios/s
pc-bios/s390-ccw/netboot.mak: Ignore Clang's warnings about GNU extensions
When compiling the s390-ccw bios with Clang (v14.0), there is currently an unuseful warning like this:
CC pc-bios/s390-ccw/ipv6.o ../../roms/SLOF/lib/libnet/ipv6.c:447:18: warning: variable length array folded to constant array as an extension [-Wgnu-folding-constant] unsigned short raw[ip6size]; ^
SLOF is currently GCC-only and cannot be compiled with Clang yet, so it is expected that such extensions sneak in there - and as long as we don't want to compile the code with a compiler that is neither GCC or Clang, it is also not necessary to avoid such extensions.
Thus these GNU-extension related warnings are completely useless in the s390-ccw bios, especially in the code that is coming from SLOF, so we should simply disable the related warnings here now.
Message-Id: <20220704111903.62400-13-thuth@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|
3953ae18 | 04-Jul-2022 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw/virtio: Remove "extern" keyword from prototypes
All the other protytpes in the headers here do not use the "extern" keyword, so let's unify this by removing the "extern" from the mi
pc-bios/s390-ccw/virtio: Remove "extern" keyword from prototypes
All the other protytpes in the headers here do not use the "extern" keyword, so let's unify this by removing the "extern" from the misfits, too.
Message-Id: <20220704111903.62400-12-thuth@redhat.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|
9125a314 | 04-Jul-2022 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw/virtio-blkdev: Request the right feature bits
The virtio-blk code uses the block size and geometry fields in the config area. According to the virtio-spec, these have to be negotiat
pc-bios/s390-ccw/virtio-blkdev: Request the right feature bits
The virtio-blk code uses the block size and geometry fields in the config area. According to the virtio-spec, these have to be negotiated with the right feature bits during initialization, otherwise they might not be available. QEMU is so far very forgiving and always provides them, but we should not rely on this behavior, so let's better request them properly via the VIRTIO_BLK_F_GEOMETRY and VIRTIO_BLK_F_BLK_SIZE feature bits.
Message-Id: <20220704111903.62400-11-thuth@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|
cf30b7c4 | 04-Jul-2022 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw: Split virtio-scsi code from virtio_blk_setup_device()
The next patch is going to add more virtio-block specific code to virtio_blk_setup_device(), and if the virtio-scsi code is al
pc-bios/s390-ccw: Split virtio-scsi code from virtio_blk_setup_device()
The next patch is going to add more virtio-block specific code to virtio_blk_setup_device(), and if the virtio-scsi code is also in there, this is more cumbersome. And the calling function virtio_setup() in main.c looks at the device type already anyway, so it's more logical to separate the virtio-scsi stuff into a new function in virtio-scsi.c instead.
Message-Id: <20220704111903.62400-10-thuth@redhat.com> Reviewed-by: Eric Farman <farman@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|
07082488 | 04-Jul-2022 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw/virtio: Beautify the code for reading virtqueue configuration
It looks nicer if we separate the run_ccw() from the IPL_assert() statement, and the error message should talk about "v
pc-bios/s390-ccw/virtio: Beautify the code for reading virtqueue configuration
It looks nicer if we separate the run_ccw() from the IPL_assert() statement, and the error message should talk about "virtio device" instead of "block device", since this code is nowadays used for non-block (i.e. network) devices, too.
Message-Id: <20220704111903.62400-9-thuth@redhat.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Eric Farman <farman@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|
aa5c69ce | 04-Jul-2022 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw/virtio: Read device config after feature negotiation
Feature negotiation should be done first, since some fields in the config area can depend on the negotiated features and thus sh
pc-bios/s390-ccw/virtio: Read device config after feature negotiation
Feature negotiation should be done first, since some fields in the config area can depend on the negotiated features and thus should rather be read afterwards.
While we're at it, also adjust the error message here a little bit (the code is nowadays used for non-block virtio devices, too).
Message-Id: <20220704111903.62400-8-thuth@redhat.com> Reviewed-by: Eric Farman <farman@linux.ibm.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|
175aa06a | 04-Jul-2022 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw/virtio: Set missing status bits while initializing
According chapter "3.1.1 Driver Requirements: Device Initialization" of the Virtio specification (v1.1), a driver for a device has
pc-bios/s390-ccw/virtio: Set missing status bits while initializing
According chapter "3.1.1 Driver Requirements: Device Initialization" of the Virtio specification (v1.1), a driver for a device has to set the ACKNOWLEDGE and DRIVER bits in the status field after resetting the device. The s390-ccw bios skipped these steps so far and seems like QEMU never cared. Anyway, it's better to follow the spec, so let's set these bits now in the right spots, too.
Message-Id: <20220704111903.62400-7-thuth@redhat.com> Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Eric Farman <farman@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|
5447de26 | 04-Jul-2022 |
Thomas Huth <thuth@redhat.com> |
pc-bios/s390-ccw/virtio-blkdev: Remove virtio_assume_scsi()
The virtio_assume_scsi() function is very questionable: First, it is only called for virtio-blk, and not for virtio-scsi, so the naming is
pc-bios/s390-ccw/virtio-blkdev: Remove virtio_assume_scsi()
The virtio_assume_scsi() function is very questionable: First, it is only called for virtio-blk, and not for virtio-scsi, so the naming is already quite confusing. Second, it is called if we detected a "invalid" IPL disk, trying to fix it by blindly setting a sector size of 512. This of course won't work in most cases since disks might have a different sector size for a reason.
Thus let's remove this strange function now. The calling code can also be removed completely, since there is another spot in main.c that does "IPL_assert(virtio_ipl_disk_is_valid(), ...)" to make sure that we do not try to IPL from an invalid device.
Message-Id: <20220704111903.62400-6-thuth@redhat.com> Reviewed-by: Eric Farman <farman@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
show more ...
|