Re: [Qemu-devel] [PATCH v8 3/4] fdc: add function to determine drive chs limits

[Michael S. Tsirkin]

On Wed, Feb 17, 2016 at 09:25:32PM +0300, Roman Kagan wrote:
> When populating ACPI objects for floppy drives one needs to provide the
> maximum values for cylinder, sector, and head number the drive supports.
> 
> This patch adds a function that iterates through the array of predefined
> floppy drive formats and returns the maximum values of c, h, s, out of
> those matching the given floppy drive type.
> 
> Signed-off-by: Roman Kagan <address@hidden>
> Cc: Igor Mammedov <address@hidden>
> Cc: "Michael S. Tsirkin" <address@hidden>
> Cc: Marcel Apfelbaum <address@hidden>
> Cc: John Snow <address@hidden>
> Cc: Laszlo Ersek <address@hidden>
> Cc: Kevin O'Connor <address@hidden>
> ---
> changes since v7:
>  - use drive max c,h,s rather than the current diskette geometry
> 
>  hw/block/fdc.c         | 23 +++++++++++++++++++++++
>  include/hw/block/fdc.h |  2 ++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 9838d21..fc3aef9 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -2557,6 +2557,29 @@ FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, 
> int i)
>      return isa->state.drives[i].drive;
>  }
>  
> +void isa_fdc_get_drive_max_chs(FloppyDriveType type,
> +                               uint8_t *maxc, uint8_t *maxh, uint8_t *maxs)
> +{
> +    const FDFormat *fdf;
> +
> +    *maxc = *maxh = *maxs = 0;
> +    for (fdf = fd_formats; fdf->drive != FLOPPY_DRIVE_TYPE_NONE; fdf++) {
> +        if (fdf->drive != type) {
> +            continue;
> +        }

Hmm. How does this interact with the fallback/autodetect thing?

I understand what it does rather vaguely.

I wonder whether we can just ignore the type and take
global maximum in all cases.

> +        if (*maxc < fdf->max_track) {
> +            *maxc = fdf->max_track;
> +        }
> +        if (*maxh < fdf->max_head) {
> +            *maxh = fdf->max_head;
> +        }
> +        if (*maxs < fdf->last_sect) {
> +            *maxs = fdf->last_sect;
> +        }
> +    }
> +    (*maxc)--;

Why not just *maxc = fdf->max_track - 1 above?

> +}
> +
>  static const VMStateDescription vmstate_isa_fdc ={
>      .name = "fdc",
>      .version_id = 2,
> diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
> index adce14f..1749dab 100644
> --- a/include/hw/block/fdc.h
> +++ b/include/hw/block/fdc.h
> @@ -15,5 +15,7 @@ void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
>                         DriveInfo **fds, qemu_irq *fdc_tc);
>  
>  FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
> +void isa_fdc_get_drive_max_chs(FloppyDriveType type,
> +                               uint8_t *maxc, uint8_t *maxh, uint8_t *maxs);
>  
>  #endif
> -- 
> 2.5.0