Sure. Thanks. Please let me know if we have something else stalled.
Den On 7/30/26 15:04, Markus Armbruster wrote: > Denis, would you be interested? > > Peter Maydell <[email protected]> writes: > >> Ping^2 ? >> >> thanks >> -- PMM >> >> On Mon, 20 Jul 2026 at 09:42, Peter Maydell <[email protected]> wrote: >>> Ping -- would anybody on the qemu-block side like to review this? >>> >>> thanks >>> -- PMM >>> >>> On Tue, 30 Jun 2026 at 11:42, Peter Maydell <[email protected]> >>> wrote: >>>> In ide_set_sector() if we're using CHS addressing the way we update >>>> the drive state involves dividing by (s->heads * s->sectors). These >>>> values can be set by the guest using the INITIALIZE DRIVE PARAMETERS >>>> command, which means that s->sectors can be 0. (s->heads can't be 0 >>>> because the command passes a heads-1 value.) INITIALIZE DRIVE >>>> PARAMETERS is specified to not check the input values for validity; >>>> instead no error is posted until some other command makes an illegal >>>> access. So we have to cope with a 0 divisor here. >>>> >>>> Special-case s->sectors being zero. >>>> >>>> Cc: [email protected] >>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2399 >>>> Signed-off-by: Peter Maydell <[email protected]> >>>> --- >>>> We don't seem to ever actually check that the guest's read/write ops >>>> are within the CHS limits it sets with INITIALIZE DRIVE PARAMETERS -- >>>> ide_sect_range_ok() only checks against the underlying block device >>>> geometry. Maybe we should? Then a zero-sectors misconfig would >>>> result in our reporting the error on the read/write as the spec >>>> suggests we should. This patch fixes the division-by-zero, at least, >>>> though. >>>> --- >>>> hw/ide/core.c | 28 +++++++++++++++++++++------- >>>> 1 file changed, 21 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/hw/ide/core.c b/hw/ide/core.c >>>> index f78b00220b..b030531534 100644 >>>> --- a/hw/ide/core.c >>>> +++ b/hw/ide/core.c >>>> @@ -646,13 +646,27 @@ void ide_set_sector(IDEState *s, int64_t sector_num) >>>> } >>>> } else { >>>> /* CHS */ >>>> - cyl = sector_num / (s->heads * s->sectors); >>>> - r = sector_num % (s->heads * s->sectors); >>>> - s->hcyl = cyl >> 8; >>>> - s->lcyl = cyl; >>>> - s->select = (s->select & ~(ATA_DEV_HS)) | >>>> - ((r / s->sectors) & (ATA_DEV_HS)); >>>> - s->sector = (r % s->sectors) + 1; >>>> + if (s->sectors == 0) { >>>> + /* >>>> + * s->sectors is under guest control via INITIALIZE DRIVE >>>> + * PARAMETERS and can be 0; this is an error but can't >>>> + * be reported at INITIALIZE DRIVE PARAMETERS time so we >>>> + * have to cope with the bad value here. >>>> + * (s->heads is always at least 1.) >>>> + */ >>>> + s->hcyl = 0; >>>> + s->lcyl = 0; >>>> + s->select = s->select & ~(ATA_DEV_HS); >>>> + s->sector = 1; >>>> + } else { >>>> + cyl = sector_num / (s->heads * s->sectors); >>>> + r = sector_num % (s->heads * s->sectors); >>>> + s->hcyl = cyl >> 8; >>>> + s->lcyl = cyl; >>>> + s->select = (s->select & ~(ATA_DEV_HS)) | >>>> + ((r / s->sectors) & (ATA_DEV_HS)); >>>> + s->sector = (r % s->sectors) + 1; >>>> + } >>>> } >>>> }
