On 27/06/14 13:45, Alexander Graf wrote: > > On 27.06.14 13:25, Cornelia Huck wrote: >> From: "Eugene (jno) Dvurechenski" <j...@linux.vnet.ibm.com> >> >> Use the virtio device's configuration to figure out the disk geometry >> and use a sector size based upon the layout. >> >> [CH: s/SECTOR_SIZE/MAX_SECTOR_SIZE/g] >> Acked-by: Christian Borntraeger <borntrae...@de.ibm.com> >> Signed-off-by: Eugene (jno) Dvurechenski <j...@linux.vnet.ibm.com> >> Signed-off-by: Jens Freimann <jf...@linux.vnet.ibm.com> >> Signed-off-by: Cornelia Huck <cornelia.h...@de.ibm.com> >> --- >> pc-bios/s390-ccw/bootmap.c | 12 +++--- >> pc-bios/s390-ccw/s390-ccw.h | 2 +- >> pc-bios/s390-ccw/virtio.c | 96 >> ++++++++++++++++++++++++++++++++++++++++--- >> pc-bios/s390-ccw/virtio.h | 48 ++++++++++++++++++++++ >> 4 files changed, 147 insertions(+), 11 deletions(-) >> >> diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c >> index c216030..fa2ca26 100644 >> --- a/pc-bios/s390-ccw/bootmap.c >> +++ b/pc-bios/s390-ccw/bootmap.c >> @@ -10,6 +10,7 @@ >> #include "s390-ccw.h" >> #include "bootmap.h" >> +#include "virtio.h" >> /* #define DEBUG_FALLBACK */ >> @@ -22,7 +23,8 @@ >> #endif >> /* Scratch space */ >> -static uint8_t sec[SECTOR_SIZE] __attribute__((__aligned__(SECTOR_SIZE))); >> +static uint8_t sec[MAX_SECTOR_SIZE] >> +__attribute__((__aligned__(MAX_SECTOR_SIZE))); >> typedef struct ResetInfo { >> uint32_t ipl_mask; >> @@ -99,7 +101,7 @@ static inline bool unused_space(const void *p, unsigned >> int size) >> static int zipl_load_segment(ComponentEntry *entry) >> { >> - const int max_entries = (SECTOR_SIZE / sizeof(ScsiBlockPtr)); >> + const int max_entries = (MAX_SECTOR_SIZE / sizeof(ScsiBlockPtr)); > > Is this really safe to increase? Doesn't max_entries depend on the real > sector size?
I think this is now covered by this if statement: if (bprs[i].blockct == 0 && unused_space(&bprs[i + 1], sizeof(ScsiBlockPtr))) { which was introduced by commit c77cd87cf54f003748f29c14ea1ddaecfc5c653f (pc-bios/s390-ccw: fix for fragmented SCSI bootmap). So strictly speaking this if statement might not be needed any more: if (i == (max_entries - 1)) { Eugene, can you confirm? If yes we could add this patch later on as a cleanup: --- a/pc-bios/s390-ccw/bootmap.c +++ b/pc-bios/s390-ccw/bootmap.c @@ -303,7 +303,6 @@ static void ipl_eckd(ECKD_IPL_mode_t mode) static void zipl_load_segment(ComponentEntry *entry) { - const int max_entries = (SECTOR_SIZE / sizeof(ScsiBlockPtr)); ScsiBlockPtr *bprs = (void *)sec; const int bprs_size = sizeof(sec); block_number_t blockno; @@ -331,12 +330,6 @@ static void zipl_load_segment(ComponentEntry *entry) break; } - /* we need the updated blockno for the next indirect entry in the - chain, but don't want to advance address */ - if (i == (max_entries - 1)) { - break; - } - if (bprs[i].blockct == 0 && unused_space(&bprs[i + 1], sizeof(ScsiBlockPtr))) { /* This is a "continue" pointer.