Add new cylinder/head/sector struct. Use it to calculate
eckd block numbers instead of a BootMapPointer (which used
eckd chs anyway).
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
---
pc-bios/s390-ccw/bootmap.c | 28 ++++++++++++++--------------
pc-bios/s390-ccw/bootmap.h | 8 ++++++--
2 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 6b6c915..621adbe 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -95,32 +95,32 @@ static inline void verify_boot_info(BootInfo *bip)
"Bad block size in zIPL section of the 1st record.");
}
-static block_number_t eckd_block_num(BootMapPointer *p)
+static block_number_t eckd_block_num(EckdCHS chs)
{
const uint64_t sectors = virtio_get_sectors();
const uint64_t heads = virtio_get_heads();
- const uint64_t cylinder = p->eckd.cylinder
- + ((p->eckd.head & 0xfff0) << 12);
- const uint64_t head = p->eckd.head & 0x000f;
+ const uint64_t cylinder = chs.cylinder
+ + ((chs.head & 0xfff0) << 12);
+ const uint64_t head = chs.head & 0x000f;
const block_number_t block = sectors * heads * cylinder
+ sectors * head
- + p->eckd.sector
+ + chs.sector
- 1; /* block nr starts with zero */
return block;
}
static bool eckd_valid_address(BootMapPointer *p)
{
- const uint64_t head = p->eckd.head & 0x000f;
+ const uint64_t head = p->eckd.chs.head & 0x000f;
if (head >= virtio_get_heads()
- || p->eckd.sector > virtio_get_sectors()
- || p->eckd.sector <= 0) {
+ || p->eckd.chs.sector > virtio_get_sectors()
+ || p->eckd.chs.sector <= 0) {
return false;
}
if (!virtio_guessed_disk_nature() &&
- eckd_block_num(p) >= virtio_get_blocks()) {
+ eckd_block_num(p->eckd.chs) >= virtio_get_blocks()) {
return false;
}
@@ -140,7 +140,7 @@ static block_number_t load_eckd_segments(block_number_t blk, uint64_t *address)
do {
more_data = false;
for (j = 0;; j++) {
- block_nr = eckd_block_num((void *)&(bprs[j].xeckd));
+ block_nr = eckd_block_num(bprs[j].xeckd.bptr.chs);
if (is_null_block_number(block_nr)) { /* end of chunk */
break;
}
@@ -198,7 +198,7 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr)
memset(sec, FREE_SPACE_FILLER, sizeof(sec));
read_block(bmt_block_nr, sec, "Cannot read Boot Map Table");
- block_nr = eckd_block_num((void *)&(bmt->bte[loadparm]));
+ block_nr = eckd_block_num(bmt->bte[loadparm].xeckd.bptr.chs);
IPL_assert(block_nr != -1, "Cannot find Boot Map Table Entry");
memset(sec, FREE_SPACE_FILLER, sizeof(sec));
@@ -206,7 +206,7 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr)
for (i = 0; bms->entry[i].type == BOOT_SCRIPT_LOAD; i++) {
address = bms->entry[i].address.load_address;
- block_nr = eckd_block_num(&(bms->entry[i].blkptr));
+ block_nr = eckd_block_num(bms->entry[i].blkptr.xeckd.bptr.chs);
do {
block_nr = load_eckd_segments(block_nr, &address);
@@ -239,7 +239,7 @@ static void ipl_eckd_cdl(void)
"Non-ECKD device type in zIPL section of IPL2 record.");
/* save pointer to Boot Map Table */
- bmt_block_nr = eckd_block_num((void *)&(mbr->blockptr));
+ bmt_block_nr = eckd_block_num(mbr->blockptr.xeckd.bptr.chs);
memset(sec, FREE_SPACE_FILLER, sizeof(sec));
read_block(2, vlbl, "Cannot read Volume Label at block 2");
@@ -300,7 +300,7 @@ static void ipl_eckd_ldl(ECKD_IPL_mode_t mode)
verify_boot_info(bip);
/* save pointer to Boot Map Table */
- bmt_block_nr = eckd_block_num((void *)&(bip->bp.ipl.bm_ptr.eckd.bptr));
+ bmt_block_nr = eckd_block_num(bip->bp.ipl.bm_ptr.eckd.bptr.chs);
run_eckd_boot_script(bmt_block_nr);
/* no return */
diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index 77d56db..260ac2a 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -32,10 +32,14 @@ typedef struct FbaBlockPtr {
uint16_t blockct;
} __attribute__ ((packed)) FbaBlockPtr;
-typedef struct EckdBlockPtr {
- uint16_t cylinder; /* cylinder/head/sector is an address of the block */
+typedef struct EckdCHS {
+ uint16_t cylinder;
uint16_t head;
uint8_t sector;
+} __attribute__ ((packed)) EckdCHS;
+
+typedef struct EckdBlockPtr {
+ EckdCHS chs; /* cylinder/head/sector is an address of the block */
uint16_t size;
uint8_t count; /* (size_in_blocks-1);
* it's 0 for TablePtr, ScriptPtr, and SectionPtr */
--
2.7.4
On 23.01.2018 19:26, Collin L. Walling wrote: > Add new cylinder/head/sector struct. Use it to calculate > eckd block numbers instead of a BootMapPointer (which used > eckd chs anyway). > > Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> > --- > pc-bios/s390-ccw/bootmap.c | 28 ++++++++++++++-------------- > pc-bios/s390-ccw/bootmap.h | 8 ++++++-- > 2 files changed, 20 insertions(+), 16 deletions(-) > > diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c > index 6b6c915..621adbe 100644 > --- a/pc-bios/s390-ccw/bootmap.c > +++ b/pc-bios/s390-ccw/bootmap.c > @@ -95,32 +95,32 @@ static inline void verify_boot_info(BootInfo *bip) > "Bad block size in zIPL section of the 1st record."); > } > > -static block_number_t eckd_block_num(BootMapPointer *p) > +static block_number_t eckd_block_num(EckdCHS chs) Should this maybe rather be call-by-pointer instead? I'm not a fan of passing structs by value, though it might be OK in this case since it's a small struct only... What do others think? Thomas PS: Apart from that, the patch looks fine to me.
On Thu, 25 Jan 2018 12:06:50 +0100 Thomas Huth <thuth@redhat.com> wrote: > On 23.01.2018 19:26, Collin L. Walling wrote: > > Add new cylinder/head/sector struct. Use it to calculate > > eckd block numbers instead of a BootMapPointer (which used > > eckd chs anyway). > > > > Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> > > --- > > pc-bios/s390-ccw/bootmap.c | 28 ++++++++++++++-------------- > > pc-bios/s390-ccw/bootmap.h | 8 ++++++-- > > 2 files changed, 20 insertions(+), 16 deletions(-) > > > > diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c > > index 6b6c915..621adbe 100644 > > --- a/pc-bios/s390-ccw/bootmap.c > > +++ b/pc-bios/s390-ccw/bootmap.c > > @@ -95,32 +95,32 @@ static inline void verify_boot_info(BootInfo *bip) > > "Bad block size in zIPL section of the 1st record."); > > } > > > > -static block_number_t eckd_block_num(BootMapPointer *p) > > +static block_number_t eckd_block_num(EckdCHS chs) > > Should this maybe rather be call-by-pointer instead? I'm not a fan of > passing structs by value, though it might be OK in this case since it's > a small struct only... > > What do others think? I think passing a struct by value is fine for things like a schid (which is basically just the structured version of an integer). In this case, I think passing a pointer would look nicer.
On 01/25/2018 06:17 AM, Cornelia Huck wrote: > On Thu, 25 Jan 2018 12:06:50 +0100 > Thomas Huth <thuth@redhat.com> wrote: > >> On 23.01.2018 19:26, Collin L. Walling wrote: >>> Add new cylinder/head/sector struct. Use it to calculate >>> eckd block numbers instead of a BootMapPointer (which used >>> eckd chs anyway). >>> >>> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> >>> --- >>> pc-bios/s390-ccw/bootmap.c | 28 ++++++++++++++-------------- >>> pc-bios/s390-ccw/bootmap.h | 8 ++++++-- >>> 2 files changed, 20 insertions(+), 16 deletions(-) >>> >>> diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c >>> index 6b6c915..621adbe 100644 >>> --- a/pc-bios/s390-ccw/bootmap.c >>> +++ b/pc-bios/s390-ccw/bootmap.c >>> @@ -95,32 +95,32 @@ static inline void verify_boot_info(BootInfo *bip) >>> "Bad block size in zIPL section of the 1st record."); >>> } >>> >>> -static block_number_t eckd_block_num(BootMapPointer *p) >>> +static block_number_t eckd_block_num(EckdCHS chs) >> Should this maybe rather be call-by-pointer instead? I'm not a fan of >> passing structs by value, though it might be OK in this case since it's >> a small struct only... >> >> What do others think? > I think passing a struct by value is fine for things like a schid > (which is basically just the structured version of an integer). In this > case, I think passing a pointer would look nicer. > Easy enough :) -- - Collin L Walling
© 2016 - 2025 Red Hat, Inc.