* [PATCH v3 1/7] s390-ccw: Getting rid of ulong
2023-06-29 10:48 [PATCH v3 0/7] pc-bios/s390-ccw: Fixes and improvements for start.S (and other files) Thomas Huth
@ 2023-06-29 10:48 ` Thomas Huth
2023-06-29 11:01 ` Claudio Imbrenda
2023-06-29 11:12 ` Philippe Mathieu-Daudé
2023-06-29 10:48 ` [PATCH v3 2/7] pc-bios/s390-ccw: Get rid of the the __u* types Thomas Huth
` (5 subsequent siblings)
6 siblings, 2 replies; 13+ messages in thread
From: Thomas Huth @ 2023-06-29 10:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-s390x, Christian Borntraeger, Eric Farman, Claudio Imbrenda
From: Juan Quintela <quintela@redhat.com>
Any good reason why this still exist?
I can understand u* and __u* to be linux kernel like, but ulong?
Signed-off-by: Juan Quintela <quintela@redhat.com>
Message-Id: <20230510143925.4094-4-quintela@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
pc-bios/s390-ccw/helper.h | 2 +-
pc-bios/s390-ccw/s390-ccw.h | 7 +++----
pc-bios/s390-ccw/virtio-scsi.h | 2 +-
pc-bios/s390-ccw/virtio.h | 4 ++--
pc-bios/s390-ccw/virtio-blkdev.c | 12 ++++++------
pc-bios/s390-ccw/virtio-scsi.c | 4 ++--
pc-bios/s390-ccw/virtio.c | 12 ++++++------
7 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/pc-bios/s390-ccw/helper.h b/pc-bios/s390-ccw/helper.h
index 3d0731c4c6..8e3dfcb6d6 100644
--- a/pc-bios/s390-ccw/helper.h
+++ b/pc-bios/s390-ccw/helper.h
@@ -38,7 +38,7 @@ static inline void yield(void)
static inline void sleep(unsigned int seconds)
{
- ulong target = get_time_seconds() + seconds;
+ unsigned long target = get_time_seconds() + seconds;
while (get_time_seconds() < target) {
yield();
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index b88e0550ab..f849fba74b 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -17,7 +17,6 @@ typedef unsigned char u8;
typedef unsigned short u16;
typedef unsigned int u32;
typedef unsigned long long u64;
-typedef unsigned long ulong;
typedef unsigned char __u8;
typedef unsigned short __u16;
typedef unsigned int __u32;
@@ -67,11 +66,11 @@ void sclp_get_loadparm_ascii(char *loadparm);
int sclp_read(char *str, size_t count);
/* virtio.c */
-unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2,
- ulong subchan_id, void *load_addr);
+unsigned long virtio_load_direct(unsigned long rec_list1, unsigned long rec_list2,
+ unsigned long subchan_id, void *load_addr);
bool virtio_is_supported(SubChannelId schid);
int virtio_blk_setup_device(SubChannelId schid);
-int virtio_read(ulong sector, void *load_addr);
+int virtio_read(unsigned long sector, void *load_addr);
/* bootmap.c */
void zipl_load(void);
diff --git a/pc-bios/s390-ccw/virtio-scsi.h b/pc-bios/s390-ccw/virtio-scsi.h
index e6b6cd4815..c5612e16a2 100644
--- a/pc-bios/s390-ccw/virtio-scsi.h
+++ b/pc-bios/s390-ccw/virtio-scsi.h
@@ -68,7 +68,7 @@ static inline bool virtio_scsi_response_ok(const VirtioScsiCmdResp *r)
}
int virtio_scsi_read_many(VDev *vdev,
- ulong sector, void *load_addr, int sec_num);
+ unsigned long sector, void *load_addr, int sec_num);
int virtio_scsi_setup_device(SubChannelId schid);
#endif /* VIRTIO_SCSI_H */
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index e657d381ec..85bd9d1695 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -190,14 +190,14 @@ int virtio_get_block_size(void);
uint8_t virtio_get_heads(void);
uint8_t virtio_get_sectors(void);
uint64_t virtio_get_blocks(void);
-int virtio_read_many(ulong sector, void *load_addr, int sec_num);
+int virtio_read_many(unsigned long sector, void *load_addr, int sec_num);
#define VIRTIO_SECTOR_SIZE 512
#define VIRTIO_ISO_BLOCK_SIZE 2048
#define VIRTIO_SCSI_BLOCK_SIZE 512
#define VIRTIO_DASD_DEFAULT_BLOCK_SIZE 4096
-static inline ulong virtio_sector_adjust(ulong sector)
+static inline unsigned long virtio_sector_adjust(unsigned long sector)
{
return sector * (virtio_get_block_size() / VIRTIO_SECTOR_SIZE);
}
diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c
index 794f99b42c..a81207b52e 100644
--- a/pc-bios/s390-ccw/virtio-blkdev.c
+++ b/pc-bios/s390-ccw/virtio-blkdev.c
@@ -16,7 +16,7 @@
#define VIRTIO_BLK_F_GEOMETRY (1 << 4)
#define VIRTIO_BLK_F_BLK_SIZE (1 << 6)
-static int virtio_blk_read_many(VDev *vdev, ulong sector, void *load_addr,
+static int virtio_blk_read_many(VDev *vdev, unsigned long sector, void *load_addr,
int sec_num)
{
VirtioBlkOuthdr out_hdr;
@@ -49,7 +49,7 @@ static int virtio_blk_read_many(VDev *vdev, ulong sector, void *load_addr,
return status;
}
-int virtio_read_many(ulong sector, void *load_addr, int sec_num)
+int virtio_read_many(unsigned long sector, void *load_addr, int sec_num)
{
VDev *vdev = virtio_get_device();
@@ -63,14 +63,14 @@ int virtio_read_many(ulong sector, void *load_addr, int sec_num)
return -1;
}
-unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2,
- ulong subchan_id, void *load_addr)
+unsigned long virtio_load_direct(unsigned long rec_list1, unsigned long rec_list2,
+ unsigned long subchan_id, void *load_addr)
{
u8 status;
int sec = rec_list1;
int sec_num = ((rec_list2 >> 32) & 0xffff) + 1;
int sec_len = rec_list2 >> 48;
- ulong addr = (ulong)load_addr;
+ unsigned long addr = (unsigned long)load_addr;
if (sec_len != virtio_get_block_size()) {
return -1;
@@ -86,7 +86,7 @@ unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2,
return addr;
}
-int virtio_read(ulong sector, void *load_addr)
+int virtio_read(unsigned long sector, void *load_addr)
{
return virtio_read_many(sector, load_addr, 1);
}
diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
index dcce696a33..d1a84b937c 100644
--- a/pc-bios/s390-ccw/virtio-scsi.c
+++ b/pc-bios/s390-ccw/virtio-scsi.c
@@ -150,7 +150,7 @@ static bool scsi_report_luns(VDev *vdev, void *data, uint32_t data_size)
}
static bool scsi_read_10(VDev *vdev,
- ulong sector, int sectors, void *data,
+ unsigned long sector, int sectors, void *data,
unsigned int data_size)
{
ScsiCdbRead10 cdb = {
@@ -269,7 +269,7 @@ static int virtio_scsi_locate_device(VDev *vdev)
}
int virtio_scsi_read_many(VDev *vdev,
- ulong sector, void *load_addr, int sec_num)
+ unsigned long sector, void *load_addr, int sec_num)
{
int sector_count;
int f = vdev->blk_factor;
diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
index f37510f312..5edd058d88 100644
--- a/pc-bios/s390-ccw/virtio.c
+++ b/pc-bios/s390-ccw/virtio.c
@@ -48,10 +48,10 @@ VirtioDevType virtio_get_device_type(void)
static long kvm_hypercall(unsigned long nr, unsigned long param1,
unsigned long param2, unsigned long param3)
{
- register ulong r_nr asm("1") = nr;
- register ulong r_param1 asm("2") = param1;
- register ulong r_param2 asm("3") = param2;
- register ulong r_param3 asm("4") = param3;
+ register unsigned long r_nr asm("1") = nr;
+ register unsigned long r_param1 asm("2") = param1;
+ register unsigned long r_param2 asm("3") = param2;
+ register unsigned long r_param3 asm("4") = param3;
register long retval asm("2");
asm volatile ("diag %%r2,%%r4,0x500"
@@ -145,7 +145,7 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags)
vr->avail->ring[vr->avail->idx % vr->num] = vr->next_idx;
}
- vr->desc[vr->next_idx].addr = (ulong)p;
+ vr->desc[vr->next_idx].addr = (unsigned long)p;
vr->desc[vr->next_idx].len = len;
vr->desc[vr->next_idx].flags = flags & ~VRING_HIDDEN_IS_CHAIN;
vr->desc[vr->next_idx].next = vr->next_idx;
@@ -182,7 +182,7 @@ int vr_poll(VRing *vr)
*/
int vring_wait_reply(void)
{
- ulong target_second = get_time_seconds() + vdev.wait_reply_timeout;
+ unsigned long target_second = get_time_seconds() + vdev.wait_reply_timeout;
/* Wait for any queue to be updated by the host */
do {
--
2.39.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v3 1/7] s390-ccw: Getting rid of ulong
2023-06-29 10:48 ` [PATCH v3 1/7] s390-ccw: Getting rid of ulong Thomas Huth
@ 2023-06-29 11:01 ` Claudio Imbrenda
2023-06-29 11:12 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 13+ messages in thread
From: Claudio Imbrenda @ 2023-06-29 11:01 UTC (permalink / raw)
To: Thomas Huth; +Cc: qemu-devel, qemu-s390x, Christian Borntraeger, Eric Farman
On Thu, 29 Jun 2023 12:48:15 +0200
Thomas Huth <thuth@redhat.com> wrote:
> From: Juan Quintela <quintela@redhat.com>
>
> Any good reason why this still exist?
> I can understand u* and __u* to be linux kernel like, but ulong?
shorter code? ¯\_(ツ)_/¯
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Message-Id: <20230510143925.4094-4-quintela@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> pc-bios/s390-ccw/helper.h | 2 +-
> pc-bios/s390-ccw/s390-ccw.h | 7 +++----
> pc-bios/s390-ccw/virtio-scsi.h | 2 +-
> pc-bios/s390-ccw/virtio.h | 4 ++--
> pc-bios/s390-ccw/virtio-blkdev.c | 12 ++++++------
> pc-bios/s390-ccw/virtio-scsi.c | 4 ++--
> pc-bios/s390-ccw/virtio.c | 12 ++++++------
> 7 files changed, 21 insertions(+), 22 deletions(-)
>
> diff --git a/pc-bios/s390-ccw/helper.h b/pc-bios/s390-ccw/helper.h
> index 3d0731c4c6..8e3dfcb6d6 100644
> --- a/pc-bios/s390-ccw/helper.h
> +++ b/pc-bios/s390-ccw/helper.h
> @@ -38,7 +38,7 @@ static inline void yield(void)
>
> static inline void sleep(unsigned int seconds)
> {
> - ulong target = get_time_seconds() + seconds;
> + unsigned long target = get_time_seconds() + seconds;
>
> while (get_time_seconds() < target) {
> yield();
> diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
> index b88e0550ab..f849fba74b 100644
> --- a/pc-bios/s390-ccw/s390-ccw.h
> +++ b/pc-bios/s390-ccw/s390-ccw.h
> @@ -17,7 +17,6 @@ typedef unsigned char u8;
> typedef unsigned short u16;
> typedef unsigned int u32;
> typedef unsigned long long u64;
> -typedef unsigned long ulong;
> typedef unsigned char __u8;
> typedef unsigned short __u16;
> typedef unsigned int __u32;
> @@ -67,11 +66,11 @@ void sclp_get_loadparm_ascii(char *loadparm);
> int sclp_read(char *str, size_t count);
>
> /* virtio.c */
> -unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2,
> - ulong subchan_id, void *load_addr);
> +unsigned long virtio_load_direct(unsigned long rec_list1, unsigned long rec_list2,
> + unsigned long subchan_id, void *load_addr);
> bool virtio_is_supported(SubChannelId schid);
> int virtio_blk_setup_device(SubChannelId schid);
> -int virtio_read(ulong sector, void *load_addr);
> +int virtio_read(unsigned long sector, void *load_addr);
>
> /* bootmap.c */
> void zipl_load(void);
> diff --git a/pc-bios/s390-ccw/virtio-scsi.h b/pc-bios/s390-ccw/virtio-scsi.h
> index e6b6cd4815..c5612e16a2 100644
> --- a/pc-bios/s390-ccw/virtio-scsi.h
> +++ b/pc-bios/s390-ccw/virtio-scsi.h
> @@ -68,7 +68,7 @@ static inline bool virtio_scsi_response_ok(const VirtioScsiCmdResp *r)
> }
>
> int virtio_scsi_read_many(VDev *vdev,
> - ulong sector, void *load_addr, int sec_num);
> + unsigned long sector, void *load_addr, int sec_num);
> int virtio_scsi_setup_device(SubChannelId schid);
>
> #endif /* VIRTIO_SCSI_H */
> diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
> index e657d381ec..85bd9d1695 100644
> --- a/pc-bios/s390-ccw/virtio.h
> +++ b/pc-bios/s390-ccw/virtio.h
> @@ -190,14 +190,14 @@ int virtio_get_block_size(void);
> uint8_t virtio_get_heads(void);
> uint8_t virtio_get_sectors(void);
> uint64_t virtio_get_blocks(void);
> -int virtio_read_many(ulong sector, void *load_addr, int sec_num);
> +int virtio_read_many(unsigned long sector, void *load_addr, int sec_num);
>
> #define VIRTIO_SECTOR_SIZE 512
> #define VIRTIO_ISO_BLOCK_SIZE 2048
> #define VIRTIO_SCSI_BLOCK_SIZE 512
> #define VIRTIO_DASD_DEFAULT_BLOCK_SIZE 4096
>
> -static inline ulong virtio_sector_adjust(ulong sector)
> +static inline unsigned long virtio_sector_adjust(unsigned long sector)
> {
> return sector * (virtio_get_block_size() / VIRTIO_SECTOR_SIZE);
> }
> diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c
> index 794f99b42c..a81207b52e 100644
> --- a/pc-bios/s390-ccw/virtio-blkdev.c
> +++ b/pc-bios/s390-ccw/virtio-blkdev.c
> @@ -16,7 +16,7 @@
> #define VIRTIO_BLK_F_GEOMETRY (1 << 4)
> #define VIRTIO_BLK_F_BLK_SIZE (1 << 6)
>
> -static int virtio_blk_read_many(VDev *vdev, ulong sector, void *load_addr,
> +static int virtio_blk_read_many(VDev *vdev, unsigned long sector, void *load_addr,
> int sec_num)
> {
> VirtioBlkOuthdr out_hdr;
> @@ -49,7 +49,7 @@ static int virtio_blk_read_many(VDev *vdev, ulong sector, void *load_addr,
> return status;
> }
>
> -int virtio_read_many(ulong sector, void *load_addr, int sec_num)
> +int virtio_read_many(unsigned long sector, void *load_addr, int sec_num)
> {
> VDev *vdev = virtio_get_device();
>
> @@ -63,14 +63,14 @@ int virtio_read_many(ulong sector, void *load_addr, int sec_num)
> return -1;
> }
>
> -unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2,
> - ulong subchan_id, void *load_addr)
> +unsigned long virtio_load_direct(unsigned long rec_list1, unsigned long rec_list2,
> + unsigned long subchan_id, void *load_addr)
> {
> u8 status;
> int sec = rec_list1;
> int sec_num = ((rec_list2 >> 32) & 0xffff) + 1;
> int sec_len = rec_list2 >> 48;
> - ulong addr = (ulong)load_addr;
> + unsigned long addr = (unsigned long)load_addr;
>
> if (sec_len != virtio_get_block_size()) {
> return -1;
> @@ -86,7 +86,7 @@ unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2,
> return addr;
> }
>
> -int virtio_read(ulong sector, void *load_addr)
> +int virtio_read(unsigned long sector, void *load_addr)
> {
> return virtio_read_many(sector, load_addr, 1);
> }
> diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
> index dcce696a33..d1a84b937c 100644
> --- a/pc-bios/s390-ccw/virtio-scsi.c
> +++ b/pc-bios/s390-ccw/virtio-scsi.c
> @@ -150,7 +150,7 @@ static bool scsi_report_luns(VDev *vdev, void *data, uint32_t data_size)
> }
>
> static bool scsi_read_10(VDev *vdev,
> - ulong sector, int sectors, void *data,
> + unsigned long sector, int sectors, void *data,
> unsigned int data_size)
> {
> ScsiCdbRead10 cdb = {
> @@ -269,7 +269,7 @@ static int virtio_scsi_locate_device(VDev *vdev)
> }
>
> int virtio_scsi_read_many(VDev *vdev,
> - ulong sector, void *load_addr, int sec_num)
> + unsigned long sector, void *load_addr, int sec_num)
> {
> int sector_count;
> int f = vdev->blk_factor;
> diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
> index f37510f312..5edd058d88 100644
> --- a/pc-bios/s390-ccw/virtio.c
> +++ b/pc-bios/s390-ccw/virtio.c
> @@ -48,10 +48,10 @@ VirtioDevType virtio_get_device_type(void)
> static long kvm_hypercall(unsigned long nr, unsigned long param1,
> unsigned long param2, unsigned long param3)
> {
> - register ulong r_nr asm("1") = nr;
> - register ulong r_param1 asm("2") = param1;
> - register ulong r_param2 asm("3") = param2;
> - register ulong r_param3 asm("4") = param3;
> + register unsigned long r_nr asm("1") = nr;
> + register unsigned long r_param1 asm("2") = param1;
> + register unsigned long r_param2 asm("3") = param2;
> + register unsigned long r_param3 asm("4") = param3;
> register long retval asm("2");
>
> asm volatile ("diag %%r2,%%r4,0x500"
> @@ -145,7 +145,7 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags)
> vr->avail->ring[vr->avail->idx % vr->num] = vr->next_idx;
> }
>
> - vr->desc[vr->next_idx].addr = (ulong)p;
> + vr->desc[vr->next_idx].addr = (unsigned long)p;
> vr->desc[vr->next_idx].len = len;
> vr->desc[vr->next_idx].flags = flags & ~VRING_HIDDEN_IS_CHAIN;
> vr->desc[vr->next_idx].next = vr->next_idx;
> @@ -182,7 +182,7 @@ int vr_poll(VRing *vr)
> */
> int vring_wait_reply(void)
> {
> - ulong target_second = get_time_seconds() + vdev.wait_reply_timeout;
> + unsigned long target_second = get_time_seconds() + vdev.wait_reply_timeout;
>
> /* Wait for any queue to be updated by the host */
> do {
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 1/7] s390-ccw: Getting rid of ulong
2023-06-29 10:48 ` [PATCH v3 1/7] s390-ccw: Getting rid of ulong Thomas Huth
2023-06-29 11:01 ` Claudio Imbrenda
@ 2023-06-29 11:12 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-06-29 11:12 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: qemu-s390x, Christian Borntraeger, Eric Farman, Claudio Imbrenda
On 29/6/23 12:48, Thomas Huth wrote:
> From: Juan Quintela <quintela@redhat.com>
>
> Any good reason why this still exist?
> I can understand u* and __u* to be linux kernel like, but ulong?
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Message-Id: <20230510143925.4094-4-quintela@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> pc-bios/s390-ccw/helper.h | 2 +-
> pc-bios/s390-ccw/s390-ccw.h | 7 +++----
> pc-bios/s390-ccw/virtio-scsi.h | 2 +-
> pc-bios/s390-ccw/virtio.h | 4 ++--
> pc-bios/s390-ccw/virtio-blkdev.c | 12 ++++++------
> pc-bios/s390-ccw/virtio-scsi.c | 4 ++--
> pc-bios/s390-ccw/virtio.c | 12 ++++++------
> 7 files changed, 21 insertions(+), 22 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 2/7] pc-bios/s390-ccw: Get rid of the the __u* types
2023-06-29 10:48 [PATCH v3 0/7] pc-bios/s390-ccw: Fixes and improvements for start.S (and other files) Thomas Huth
2023-06-29 10:48 ` [PATCH v3 1/7] s390-ccw: Getting rid of ulong Thomas Huth
@ 2023-06-29 10:48 ` Thomas Huth
2023-06-29 10:48 ` [PATCH v3 3/7] pc-bios/s390-ccw/Makefile: Use -z noexecstack to silence linker warning Thomas Huth
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2023-06-29 10:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-s390x, Christian Borntraeger, Eric Farman, Claudio Imbrenda
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>
---
pc-bios/s390-ccw/cio.h | 232 ++++++++++++++++++------------------
pc-bios/s390-ccw/s390-ccw.h | 4 -
2 files changed, 116 insertions(+), 120 deletions(-)
diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
index 88a88adfd2..8b18153deb 100644
--- a/pc-bios/s390-ccw/cio.h
+++ b/pc-bios/s390-ccw/cio.h
@@ -17,32 +17,32 @@
* path management control word
*/
struct pmcw {
- __u32 intparm; /* interruption parameter */
- __u32 qf:1; /* qdio facility */
- __u32 w:1;
- __u32 isc:3; /* interruption subclass */
- __u32 res5:3; /* reserved zeros */
- __u32 ena:1; /* enabled */
- __u32 lm:2; /* limit mode */
- __u32 mme:2; /* measurement-mode enable */
- __u32 mp:1; /* multipath mode */
- __u32 tf:1; /* timing facility */
- __u32 dnv:1; /* device number valid */
- __u32 dev:16; /* device number */
- __u8 lpm; /* logical path mask */
- __u8 pnom; /* path not operational mask */
- __u8 lpum; /* last path used mask */
- __u8 pim; /* path installed mask */
- __u16 mbi; /* measurement-block index */
- __u8 pom; /* path operational mask */
- __u8 pam; /* path available mask */
- __u8 chpid[8]; /* CHPID 0-7 (if available) */
- __u32 unused1:8; /* reserved zeros */
- __u32 st:3; /* subchannel type */
- __u32 unused2:18; /* reserved zeros */
- __u32 mbfc:1; /* measurement block format control */
- __u32 xmwme:1; /* extended measurement word mode enable */
- __u32 csense:1; /* concurrent sense; can be enabled ...*/
+ u32 intparm; /* interruption parameter */
+ u32 qf:1; /* qdio facility */
+ u32 w:1;
+ u32 isc:3; /* interruption subclass */
+ u32 res5:3; /* reserved zeros */
+ u32 ena:1; /* enabled */
+ u32 lm:2; /* limit mode */
+ u32 mme:2; /* measurement-mode enable */
+ u32 mp:1; /* multipath mode */
+ u32 tf:1; /* timing facility */
+ u32 dnv:1; /* device number valid */
+ u32 dev:16; /* device number */
+ u8 lpm; /* logical path mask */
+ u8 pnom; /* path not operational mask */
+ u8 lpum; /* last path used mask */
+ u8 pim; /* path installed mask */
+ u16 mbi; /* measurement-block index */
+ u8 pom; /* path operational mask */
+ u8 pam; /* path available mask */
+ u8 chpid[8]; /* CHPID 0-7 (if available) */
+ u32 unused1:8; /* reserved zeros */
+ u32 st:3; /* subchannel type */
+ u32 unused2:18; /* reserved zeros */
+ u32 mbfc:1; /* measurement block format control */
+ u32 xmwme:1; /* extended measurement word mode enable */
+ u32 csense:1; /* concurrent sense; can be enabled ...*/
/* ... per MSCH, however, if facility */
/* ... is not installed, this results */
/* ... in an operand exception. */
@@ -50,24 +50,24 @@ struct pmcw {
/* Target SCHIB configuration. */
struct schib_config {
- __u64 mba;
- __u32 intparm;
- __u16 mbi;
- __u32 isc:3;
- __u32 ena:1;
- __u32 mme:2;
- __u32 mp:1;
- __u32 csense:1;
- __u32 mbfc:1;
+ u64 mba;
+ u32 intparm;
+ u16 mbi;
+ u32 isc:3;
+ u32 ena:1;
+ u32 mme:2;
+ u32 mp:1;
+ u32 csense:1;
+ u32 mbfc:1;
} __attribute__ ((packed));
struct scsw {
- __u16 flags;
- __u16 ctrl;
- __u32 cpa;
- __u8 dstat;
- __u8 cstat;
- __u16 count;
+ u16 flags;
+ u16 ctrl;
+ u32 cpa;
+ u8 dstat;
+ u8 cstat;
+ u16 count;
} __attribute__ ((packed));
/* Function Control */
@@ -117,42 +117,42 @@ struct scsw {
typedef struct schib {
struct pmcw pmcw; /* path management control word */
struct scsw scsw; /* subchannel status word */
- __u64 mba; /* measurement block address */
- __u8 mda[4]; /* model dependent area */
+ u64 mba; /* measurement block address */
+ u8 mda[4]; /* model dependent area */
} __attribute__ ((packed, aligned(4))) Schib;
typedef struct subchannel_id {
union {
struct {
- __u16 cssid:8;
- __u16 reserved:4;
- __u16 m:1;
- __u16 ssid:2;
- __u16 one:1;
+ u16 cssid:8;
+ u16 reserved:4;
+ u16 m:1;
+ u16 ssid:2;
+ u16 one:1;
};
- __u16 sch_id;
+ u16 sch_id;
};
- __u16 sch_no;
+ u16 sch_no;
} __attribute__ ((packed, aligned(4))) SubChannelId;
struct chsc_header {
- __u16 length;
- __u16 code;
+ u16 length;
+ u16 code;
} __attribute__((packed));
typedef struct chsc_area_sda {
struct chsc_header request;
- __u8 reserved1:4;
- __u8 format:4;
- __u8 reserved2;
- __u16 operation_code;
- __u32 reserved3;
- __u32 reserved4;
- __u32 operation_data_area[252];
+ u8 reserved1:4;
+ u8 format:4;
+ u8 reserved2;
+ u16 operation_code;
+ u32 reserved3;
+ u32 reserved4;
+ u32 operation_data_area[252];
struct chsc_header response;
- __u32 reserved5:4;
- __u32 format2:4;
- __u32 reserved6:24;
+ u32 reserved5:4;
+ u32 format2:4;
+ u32 reserved6:24;
} __attribute__((packed)) ChscAreaSda;
/*
@@ -160,37 +160,37 @@ typedef struct chsc_area_sda {
*/
struct tpi_info {
struct subchannel_id schid;
- __u32 intparm; /* interruption parameter */
- __u32 adapter_IO:1;
- __u32 reserved2:1;
- __u32 isc:3;
- __u32 reserved3:12;
- __u32 int_type:3;
- __u32 reserved4:12;
+ u32 intparm; /* interruption parameter */
+ u32 adapter_IO:1;
+ u32 reserved2:1;
+ u32 isc:3;
+ u32 reserved3:12;
+ u32 int_type:3;
+ u32 reserved4:12;
} __attribute__ ((packed, aligned(4)));
/* channel command word (format 0) */
typedef struct ccw0 {
- __u8 cmd_code;
- __u32 cda:24;
- __u32 chainData:1;
- __u32 chain:1;
- __u32 sli:1;
- __u32 skip:1;
- __u32 pci:1;
- __u32 ida:1;
- __u32 suspend:1;
- __u32 mida:1;
- __u8 reserved;
- __u16 count;
+ u8 cmd_code;
+ u32 cda:24;
+ u32 chainData:1;
+ u32 chain:1;
+ u32 sli:1;
+ u32 skip:1;
+ u32 pci:1;
+ u32 ida:1;
+ u32 suspend:1;
+ u32 mida:1;
+ u8 reserved;
+ u16 count;
} __attribute__ ((packed, aligned(8))) Ccw0;
/* channel command word (format 1) */
typedef struct ccw1 {
- __u8 cmd_code;
- __u8 flags;
- __u16 count;
- __u32 cda;
+ u8 cmd_code;
+ u8 flags;
+ u16 count;
+ u32 cda;
} __attribute__ ((packed, aligned(8))) Ccw1;
/* do_cio() CCW formats */
@@ -234,31 +234,31 @@ typedef struct ccw1 {
* Command-mode operation request block
*/
typedef struct cmd_orb {
- __u32 intparm; /* interruption parameter */
- __u32 key:4; /* flags, like key, suspend control, etc. */
- __u32 spnd:1; /* suspend control */
- __u32 res1:1; /* reserved */
- __u32 mod:1; /* modification control */
- __u32 sync:1; /* synchronize control */
- __u32 fmt:1; /* format control */
- __u32 pfch:1; /* prefetch control */
- __u32 isic:1; /* initial-status-interruption control */
- __u32 alcc:1; /* address-limit-checking control */
- __u32 ssic:1; /* suppress-suspended-interr. control */
- __u32 res2:1; /* reserved */
- __u32 c64:1; /* IDAW/QDIO 64 bit control */
- __u32 i2k:1; /* IDAW 2/4kB block size control */
- __u32 lpm:8; /* logical path mask */
- __u32 ils:1; /* incorrect length */
- __u32 zero:6; /* reserved zeros */
- __u32 orbx:1; /* ORB extension control */
- __u32 cpa; /* channel program address */
+ u32 intparm; /* interruption parameter */
+ u32 key:4; /* flags, like key, suspend control, etc. */
+ u32 spnd:1; /* suspend control */
+ u32 res1:1; /* reserved */
+ u32 mod:1; /* modification control */
+ u32 sync:1; /* synchronize control */
+ u32 fmt:1; /* format control */
+ u32 pfch:1; /* prefetch control */
+ u32 isic:1; /* initial-status-interruption control */
+ u32 alcc:1; /* address-limit-checking control */
+ u32 ssic:1; /* suppress-suspended-interr. control */
+ u32 res2:1; /* reserved */
+ u32 c64:1; /* IDAW/QDIO 64 bit control */
+ u32 i2k:1; /* IDAW 2/4kB block size control */
+ u32 lpm:8; /* logical path mask */
+ u32 ils:1; /* incorrect length */
+ u32 zero:6; /* reserved zeros */
+ u32 orbx:1; /* ORB extension control */
+ u32 cpa; /* channel program address */
} __attribute__ ((packed, aligned(4))) CmdOrb;
struct ciw {
- __u8 type;
- __u8 command;
- __u16 count;
+ u8 type;
+ u8 command;
+ u16 count;
};
#define CU_TYPE_UNKNOWN 0x0000
@@ -271,12 +271,12 @@ struct ciw {
*/
typedef struct senseid {
/* common part */
- __u8 reserved; /* always 0x'FF' */
- __u16 cu_type; /* control unit type */
- __u8 cu_model; /* control unit model */
- __u16 dev_type; /* device type */
- __u8 dev_model; /* device model */
- __u8 unused; /* padding byte */
+ u8 reserved; /* always 0x'FF' */
+ u16 cu_type; /* control unit type */
+ u8 cu_model; /* control unit model */
+ u16 dev_type; /* device type */
+ u8 dev_model; /* device model */
+ u8 unused; /* padding byte */
/* extended part */
struct ciw ciw[62];
} __attribute__ ((packed, aligned(4))) SenseId;
@@ -342,9 +342,9 @@ typedef struct SenseDataEckdDasd {
/* interruption response block */
typedef struct irb {
struct scsw scsw;
- __u32 esw[5];
- __u32 ecw[8];
- __u32 emw[8];
+ u32 esw[5];
+ u32 ecw[8];
+ u32 emw[8];
} __attribute__ ((packed, aligned(4))) Irb;
/* Used for SEEK ccw commands */
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index f849fba74b..f68a832718 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -17,10 +17,6 @@ typedef unsigned char u8;
typedef unsigned short u16;
typedef unsigned int u32;
typedef unsigned long long u64;
-typedef unsigned char __u8;
-typedef unsigned short __u16;
-typedef unsigned int __u32;
-typedef unsigned long long __u64;
#define true 1
#define false 0
--
2.39.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v3 3/7] pc-bios/s390-ccw/Makefile: Use -z noexecstack to silence linker warning
2023-06-29 10:48 [PATCH v3 0/7] pc-bios/s390-ccw: Fixes and improvements for start.S (and other files) Thomas Huth
2023-06-29 10:48 ` [PATCH v3 1/7] s390-ccw: Getting rid of ulong Thomas Huth
2023-06-29 10:48 ` [PATCH v3 2/7] pc-bios/s390-ccw: Get rid of the the __u* types Thomas Huth
@ 2023-06-29 10:48 ` Thomas Huth
2023-06-29 10:48 ` [PATCH v3 4/7] pc-bios/s390-ccw: Fix indentation in start.S Thomas Huth
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2023-06-29 10:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-s390x, Christian Borntraeger, Eric Farman, Claudio Imbrenda
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>
---
pc-bios/s390-ccw/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 2e8cc015aa..acfcd1e71a 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -55,7 +55,7 @@ config-cc.mak: Makefile
$(call cc-option,-march=z900,-march=z10)) 3> config-cc.mak
-include config-cc.mak
-LDFLAGS += -Wl,-pie -nostdlib
+LDFLAGS += -Wl,-pie -nostdlib -z noexecstack
build-all: s390-ccw.img s390-netboot.img
--
2.39.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v3 4/7] pc-bios/s390-ccw: Fix indentation in start.S
2023-06-29 10:48 [PATCH v3 0/7] pc-bios/s390-ccw: Fixes and improvements for start.S (and other files) Thomas Huth
` (2 preceding siblings ...)
2023-06-29 10:48 ` [PATCH v3 3/7] pc-bios/s390-ccw/Makefile: Use -z noexecstack to silence linker warning Thomas Huth
@ 2023-06-29 10:48 ` Thomas Huth
2023-06-29 10:48 ` [PATCH v3 5/7] pc-bios/s390-ccw: Provide space for initial stack frame " Thomas Huth
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2023-06-29 10:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-s390x, Christian Borntraeger, Eric Farman, Claudio Imbrenda
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>
---
pc-bios/s390-ccw/start.S | 136 +++++++++++++++++++--------------------
1 file changed, 68 insertions(+), 68 deletions(-)
diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S
index 6072906df4..d29de09cc6 100644
--- a/pc-bios/s390-ccw/start.S
+++ b/pc-bios/s390-ccw/start.S
@@ -10,37 +10,37 @@
* directory.
*/
- .globl _start
+ .globl _start
_start:
- larl %r15, stack + 0x8000 /* Set up stack */
+ larl %r15,stack + 0x8000 /* Set up stack */
- /* clear bss */
- larl %r2, __bss_start
- larl %r3, _end
- slgr %r3, %r2 /* get sizeof bss */
- ltgr %r3,%r3 /* bss empty? */
- jz done
- aghi %r3,-1
- srlg %r4,%r3,8 /* how many 256 byte chunks? */
- ltgr %r4,%r4
- lgr %r1,%r2
- jz remainder
+ /* clear bss */
+ larl %r2,__bss_start
+ larl %r3,_end
+ slgr %r3,%r2 /* get sizeof bss */
+ ltgr %r3,%r3 /* bss empty? */
+ jz done
+ aghi %r3,-1
+ srlg %r4,%r3,8 /* how many 256 byte chunks? */
+ ltgr %r4,%r4
+ lgr %r1,%r2
+ jz remainder
loop:
- xc 0(256,%r1),0(%r1)
- la %r1,256(%r1)
- brctg %r4,loop
+ xc 0(256,%r1),0(%r1)
+ la %r1,256(%r1)
+ brctg %r4,loop
remainder:
- larl %r2,memsetxc
- ex %r3,0(%r2)
+ larl %r2,memsetxc
+ ex %r3,0(%r2)
done:
- /* set up a pgm exception disabled wait psw */
- larl %r2, disabled_wait_psw
- mvc 0x01d0(16), 0(%r2)
- j main /* And call C */
+ /* set up a pgm exception disabled wait psw */
+ larl %r2,disabled_wait_psw
+ mvc 0x01d0(16),0(%r2)
+ j main /* And call C */
memsetxc:
- xc 0(1,%r1),0(%r1)
+ xc 0(1,%r1),0(%r1)
/*
@@ -48,11 +48,11 @@ memsetxc:
*
* stops the current guest cpu.
*/
- .globl disabled_wait
+ .globl disabled_wait
disabled_wait:
- larl %r1,disabled_wait_psw
- lpswe 0(%r1)
-1: j 1b
+ larl %r1,disabled_wait_psw
+ lpswe 0(%r1)
+1: j 1b
/*
@@ -60,61 +60,61 @@ disabled_wait:
*
* eats one sclp interrupt
*/
- .globl consume_sclp_int
+ .globl consume_sclp_int
consume_sclp_int:
- /* enable service interrupts in cr0 */
- stctg %c0,%c0,0(%r15)
- oi 6(%r15),0x2
- lctlg %c0,%c0,0(%r15)
- /* prepare external call handler */
- larl %r1, external_new_code
- stg %r1, 0x1b8
- larl %r1, external_new_mask
- mvc 0x1b0(8),0(%r1)
- /* load enabled wait PSW */
- larl %r1, enabled_wait_psw
- lpswe 0(%r1)
+ /* enable service interrupts in cr0 */
+ stctg %c0,%c0,0(%r15)
+ oi 6(%r15),0x2
+ lctlg %c0,%c0,0(%r15)
+ /* prepare external call handler */
+ larl %r1,external_new_code
+ stg %r1,0x1b8
+ larl %r1,external_new_mask
+ mvc 0x1b0(8),0(%r1)
+ /* load enabled wait PSW */
+ larl %r1,enabled_wait_psw
+ lpswe 0(%r1)
/*
* void consume_io_int(void)
*
* eats one I/O interrupt
*/
- .globl consume_io_int
+ .globl consume_io_int
consume_io_int:
- /* enable I/O interrupts in cr6 */
- stctg %c6,%c6,0(%r15)
- oi 4(%r15), 0xff
- lctlg %c6,%c6,0(%r15)
- /* prepare i/o call handler */
- larl %r1, io_new_code
- stg %r1, 0x1f8
- larl %r1, io_new_mask
- mvc 0x1f0(8),0(%r1)
- /* load enabled wait PSW */
- larl %r1, enabled_wait_psw
- lpswe 0(%r1)
+ /* enable I/O interrupts in cr6 */
+ stctg %c6,%c6,0(%r15)
+ oi 4(%r15), 0xff
+ lctlg %c6,%c6,0(%r15)
+ /* prepare i/o call handler */
+ larl %r1,io_new_code
+ stg %r1,0x1f8
+ larl %r1,io_new_mask
+ mvc 0x1f0(8),0(%r1)
+ /* load enabled wait PSW */
+ larl %r1,enabled_wait_psw
+ lpswe 0(%r1)
external_new_code:
- /* disable service interrupts in cr0 */
- stctg %c0,%c0,0(%r15)
- ni 6(%r15),0xfd
- lctlg %c0,%c0,0(%r15)
- br %r14
+ /* disable service interrupts in cr0 */
+ stctg %c0,%c0,0(%r15)
+ ni 6(%r15),0xfd
+ lctlg %c0,%c0,0(%r15)
+ br %r14
io_new_code:
- /* disable I/O interrupts in cr6 */
- stctg %c6,%c6,0(%r15)
- ni 4(%r15), 0x00
- lctlg %c6,%c6,0(%r15)
- br %r14
+ /* disable I/O interrupts in cr6 */
+ stctg %c6,%c6,0(%r15)
+ ni 4(%r15),0x00
+ lctlg %c6,%c6,0(%r15)
+ br %r14
- .align 8
+ .align 8
disabled_wait_psw:
- .quad 0x0002000180000000,0x0000000000000000
+ .quad 0x0002000180000000,0x0000000000000000
enabled_wait_psw:
- .quad 0x0302000180000000,0x0000000000000000
+ .quad 0x0302000180000000,0x0000000000000000
external_new_mask:
- .quad 0x0000000180000000
+ .quad 0x0000000180000000
io_new_mask:
- .quad 0x0000000180000000
+ .quad 0x0000000180000000
--
2.39.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v3 5/7] pc-bios/s390-ccw: Provide space for initial stack frame in start.S
2023-06-29 10:48 [PATCH v3 0/7] pc-bios/s390-ccw: Fixes and improvements for start.S (and other files) Thomas Huth
` (3 preceding siblings ...)
2023-06-29 10:48 ` [PATCH v3 4/7] pc-bios/s390-ccw: Fix indentation in start.S Thomas Huth
@ 2023-06-29 10:48 ` Thomas Huth
2023-06-29 10:48 ` [PATCH v3 6/7] pc-bios/s390-ccw: Move the stack array into start.S Thomas Huth
2023-06-29 10:48 ` [PATCH v3 7/7] pc-bios/s390-ccw: Don't use __bss_start with the "larl" instruction Thomas Huth
6 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2023-06-29 10:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-s390x, Christian Borntraeger, Eric Farman, Claudio Imbrenda
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>
---
pc-bios/s390-ccw/start.S | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S
index d29de09cc6..abd6fe6639 100644
--- a/pc-bios/s390-ccw/start.S
+++ b/pc-bios/s390-ccw/start.S
@@ -10,10 +10,13 @@
* directory.
*/
+#define STACK_SIZE 0x8000
+#define STACK_FRAME_SIZE 160
+
.globl _start
_start:
- larl %r15,stack + 0x8000 /* Set up stack */
+ larl %r15,stack + STACK_SIZE - STACK_FRAME_SIZE /* Set up stack */
/* clear bss */
larl %r2,__bss_start
--
2.39.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v3 6/7] pc-bios/s390-ccw: Move the stack array into start.S
2023-06-29 10:48 [PATCH v3 0/7] pc-bios/s390-ccw: Fixes and improvements for start.S (and other files) Thomas Huth
` (4 preceding siblings ...)
2023-06-29 10:48 ` [PATCH v3 5/7] pc-bios/s390-ccw: Provide space for initial stack frame " Thomas Huth
@ 2023-06-29 10:48 ` Thomas Huth
2023-06-29 10:48 ` [PATCH v3 7/7] pc-bios/s390-ccw: Don't use __bss_start with the "larl" instruction Thomas Huth
6 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2023-06-29 10:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-s390x, Christian Borntraeger, Eric Farman, Claudio Imbrenda
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>
---
pc-bios/s390-ccw/s390-ccw.h | 1 -
pc-bios/s390-ccw/main.c | 1 -
pc-bios/s390-ccw/netmain.c | 1 -
pc-bios/s390-ccw/start.S | 6 ++++++
tests/tcg/s390x/head64.S | 7 ++-----
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index f68a832718..c977a52b50 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -50,7 +50,6 @@ void consume_io_int(void);
/* main.c */
void write_subsystem_identification(void);
void write_iplb_location(void);
-extern char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE)));
unsigned int get_loadparm_index(void);
void main(void);
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index a2def83e82..5506798098 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -17,7 +17,6 @@
#include "virtio-scsi.h"
#include "dasd-ipl.h"
-char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE)));
static SubChannelId blk_schid = { .one = 1 };
static char loadparm_str[LOADPARM_LEN + 1];
QemuIplParameters qipl;
diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
index 056e93a818..5cd619b2d6 100644
--- a/pc-bios/s390-ccw/netmain.c
+++ b/pc-bios/s390-ccw/netmain.c
@@ -50,7 +50,6 @@ void write_iplb_location(void) {}
/* STSI 3.2.2 offset of first vmdb + offset of uuid inside vmdb */
#define STSI322_VMDB_UUID_OFFSET ((8 + 12) * 4)
-char stack[PAGE_SIZE * 8] __attribute__((aligned(PAGE_SIZE)));
IplParameterBlock iplb __attribute__((aligned(PAGE_SIZE)));
static char cfgbuf[2048];
diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S
index abd6fe6639..429a2b30a1 100644
--- a/pc-bios/s390-ccw/start.S
+++ b/pc-bios/s390-ccw/start.S
@@ -121,3 +121,9 @@ external_new_mask:
.quad 0x0000000180000000
io_new_mask:
.quad 0x0000000180000000
+
+.bss
+ .align 8
+stack:
+ .space STACK_SIZE
+ .size stack,STACK_SIZE
diff --git a/tests/tcg/s390x/head64.S b/tests/tcg/s390x/head64.S
index c6f36dfea4..4fe288388a 100644
--- a/tests/tcg/s390x/head64.S
+++ b/tests/tcg/s390x/head64.S
@@ -8,6 +8,8 @@
#include "../../../pc-bios/s390-ccw/start.S"
#undef main
+.text
+
main_pre:
aghi %r15,-160 /* reserve stack for C code */
brasl %r14,sclp_setup
@@ -24,8 +26,3 @@ success_psw:
.quad 0x2000180000000,0xfff /* see is_special_wait_psw() */
failure_psw:
.quad 0x2000180000000,0 /* disabled wait */
-
- .section .bss
- .align 0x1000
-stack:
- .skip 0x8000
--
2.39.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v3 7/7] pc-bios/s390-ccw: Don't use __bss_start with the "larl" instruction
2023-06-29 10:48 [PATCH v3 0/7] pc-bios/s390-ccw: Fixes and improvements for start.S (and other files) Thomas Huth
` (5 preceding siblings ...)
2023-06-29 10:48 ` [PATCH v3 6/7] pc-bios/s390-ccw: Move the stack array into start.S Thomas Huth
@ 2023-06-29 10:48 ` Thomas Huth
2023-06-29 10:58 ` Claudio Imbrenda
6 siblings, 1 reply; 13+ messages in thread
From: Thomas Huth @ 2023-06-29 10:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-s390x, Christian Borntraeger, Eric Farman, Claudio Imbrenda
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>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
pc-bios/s390-ccw/start.S | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S
index 429a2b30a1..061b06591c 100644
--- a/pc-bios/s390-ccw/start.S
+++ b/pc-bios/s390-ccw/start.S
@@ -19,7 +19,8 @@ _start:
larl %r15,stack + STACK_SIZE - STACK_FRAME_SIZE /* Set up stack */
/* clear bss */
- larl %r2,__bss_start
+ larl %r2,bss_start_literal /* __bss_start might be unaligned ... */
+ lg %r2,0(%r2) /* ... so load it indirectly */
larl %r3,_end
slgr %r3,%r2 /* get sizeof bss */
ltgr %r3,%r3 /* bss empty? */
@@ -45,7 +46,6 @@ done:
memsetxc:
xc 0(1,%r1),0(%r1)
-
/*
* void disabled_wait(void)
*
@@ -113,6 +113,8 @@ io_new_code:
br %r14
.align 8
+bss_start_literal:
+ .quad __bss_start
disabled_wait_psw:
.quad 0x0002000180000000,0x0000000000000000
enabled_wait_psw:
--
2.39.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v3 7/7] pc-bios/s390-ccw: Don't use __bss_start with the "larl" instruction
2023-06-29 10:48 ` [PATCH v3 7/7] pc-bios/s390-ccw: Don't use __bss_start with the "larl" instruction Thomas Huth
@ 2023-06-29 10:58 ` Claudio Imbrenda
2023-06-29 11:12 ` Thomas Huth
0 siblings, 1 reply; 13+ messages in thread
From: Claudio Imbrenda @ 2023-06-29 10:58 UTC (permalink / raw)
To: Thomas Huth; +Cc: qemu-devel, qemu-s390x, Christian Borntraeger, Eric Farman
On Thu, 29 Jun 2023 12:48:21 +0200
Thomas Huth <thuth@redhat.com> wrote:
> 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.
what are the advantages of this solution compared to your previous one
(i.e. align .bss) ?
>
> 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>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> pc-bios/s390-ccw/start.S | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S
> index 429a2b30a1..061b06591c 100644
> --- a/pc-bios/s390-ccw/start.S
> +++ b/pc-bios/s390-ccw/start.S
> @@ -19,7 +19,8 @@ _start:
> larl %r15,stack + STACK_SIZE - STACK_FRAME_SIZE /* Set up stack */
>
> /* clear bss */
> - larl %r2,__bss_start
> + larl %r2,bss_start_literal /* __bss_start might be unaligned ... */
> + lg %r2,0(%r2) /* ... so load it indirectly */
> larl %r3,_end
> slgr %r3,%r2 /* get sizeof bss */
> ltgr %r3,%r3 /* bss empty? */
> @@ -45,7 +46,6 @@ done:
> memsetxc:
> xc 0(1,%r1),0(%r1)
>
> -
> /*
> * void disabled_wait(void)
> *
> @@ -113,6 +113,8 @@ io_new_code:
> br %r14
>
> .align 8
> +bss_start_literal:
> + .quad __bss_start
> disabled_wait_psw:
> .quad 0x0002000180000000,0x0000000000000000
> enabled_wait_psw:
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 7/7] pc-bios/s390-ccw: Don't use __bss_start with the "larl" instruction
2023-06-29 10:58 ` Claudio Imbrenda
@ 2023-06-29 11:12 ` Thomas Huth
2023-06-29 11:25 ` Claudio Imbrenda
0 siblings, 1 reply; 13+ messages in thread
From: Thomas Huth @ 2023-06-29 11:12 UTC (permalink / raw)
To: Claudio Imbrenda
Cc: qemu-devel, qemu-s390x, Christian Borntraeger, Eric Farman
On 29/06/2023 12.58, Claudio Imbrenda wrote:
> On Thu, 29 Jun 2023 12:48:21 +0200
> Thomas Huth <thuth@redhat.com> wrote:
>
>> 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.
>
> what are the advantages of this solution compared to your previous one
> (i.e. align .bss) ?
__bss_start is supposed to point to an address that is before all bss-like
segments. There are also segments like .sbss and .bss.plt on other
architectures, see https://bugzilla.redhat.com/show_bug.cgi?id=2216662#c11 .
Seems like we don't have them on s390x yet, so currently my previous patch
is fine, too. But in case there will ever be an extension to the s390x ABI
that introduces such additional segments, we have to switch back to
__bss_start again. So it sounds slightly more future-proof to me to keep
__bss_start here, even if we need a slightly more complex startup code here now.
Thomas
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 7/7] pc-bios/s390-ccw: Don't use __bss_start with the "larl" instruction
2023-06-29 11:12 ` Thomas Huth
@ 2023-06-29 11:25 ` Claudio Imbrenda
0 siblings, 0 replies; 13+ messages in thread
From: Claudio Imbrenda @ 2023-06-29 11:25 UTC (permalink / raw)
To: Thomas Huth; +Cc: qemu-devel, qemu-s390x, Christian Borntraeger, Eric Farman
On Thu, 29 Jun 2023 13:12:26 +0200
Thomas Huth <thuth@redhat.com> wrote:
> On 29/06/2023 12.58, Claudio Imbrenda wrote:
> > On Thu, 29 Jun 2023 12:48:21 +0200
> > Thomas Huth <thuth@redhat.com> wrote:
> >
> >> 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.
> >
> > what are the advantages of this solution compared to your previous one
> > (i.e. align .bss) ?
>
> __bss_start is supposed to point to an address that is before all bss-like
> segments. There are also segments like .sbss and .bss.plt on other
> architectures, see https://bugzilla.redhat.com/show_bug.cgi?id=2216662#c11 .
> Seems like we don't have them on s390x yet, so currently my previous patch
> is fine, too. But in case there will ever be an extension to the s390x ABI
> that introduces such additional segments, we have to switch back to
> __bss_start again. So it sounds slightly more future-proof to me to keep
> __bss_start here, even if we need a slightly more complex startup code here now.
fair enough
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>
> Thomas
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread