Re: [PATCH v5 08/15] acpi: move aml builder code for floppy device

[Gerd Hoffmann]

> > +#include "hw/i386/pc.h"
> 
> I'd rather not see this target-specific header used in the generic device
> model... The culprit seems cmos_get_fd_drive_type(). Is the value really PC
> specific?

Given that the same value is used in acpi tables probably not really.
cmos_get_fd_drive_type() hasn't any dependency on pc or cmos btw, we
can simply move over the function to floppy.

> 
> Hervé, do you know if such info & RTC nvram layout is used in other arch?
> 
> > +#include "hw/acpi/aml-build.h"
> >   #include "hw/irq.h"
> >   #include "hw/isa/isa.h"
> >   #include "hw/qdev-properties.h"
> > @@ -2765,6 +2767,85 @@ void isa_fdc_get_drive_max_chs(FloppyDriveType type,
> >       (*maxc)--;
> >   }
> > +static Aml *build_fdinfo_aml(int idx, FloppyDriveType type)
> > +{
> > +    Aml *dev, *fdi;
> > +    uint8_t maxc, maxh, maxs;
> > +
> > +    isa_fdc_get_drive_max_chs(type, &maxc, &maxh, &maxs);
> > +
> > +    dev = aml_device("FLP%c", 'A' + idx);
> > +
> > +    aml_append(dev, aml_name_decl("_ADR", aml_int(idx)));
> > +
> > +    fdi = aml_package(16);
> > +    aml_append(fdi, aml_int(idx));  /* Drive Number */
> > +    aml_append(fdi,
> > +        aml_int(cmos_get_fd_drive_type(type)));  /* Device Type */
> > +    /*
> > +     * the values below are the limits of the drive, and are thus 
> > independent
> > +     * of the inserted media
> > +     */
> > +    aml_append(fdi, aml_int(maxc));  /* Maximum Cylinder Number */
> > +    aml_append(fdi, aml_int(maxs));  /* Maximum Sector Number */
> > +    aml_append(fdi, aml_int(maxh));  /* Maximum Head Number */
> > +    /*
> > +     * SeaBIOS returns the below values for int 0x13 func 0x08 regardless 
> > of
> > +     * the drive type, so shall we
> > +     */
> > +    aml_append(fdi, aml_int(0xAF));  /* disk_specify_1 */
> > +    aml_append(fdi, aml_int(0x02));  /* disk_specify_2 */
> > +    aml_append(fdi, aml_int(0x25));  /* disk_motor_wait */
> > +    aml_append(fdi, aml_int(0x02));  /* disk_sector_siz */
> > +    aml_append(fdi, aml_int(0x12));  /* disk_eot */
> > +    aml_append(fdi, aml_int(0x1B));  /* disk_rw_gap */
> > +    aml_append(fdi, aml_int(0xFF));  /* disk_dtl */
> > +    aml_append(fdi, aml_int(0x6C));  /* disk_formt_gap */
> > +    aml_append(fdi, aml_int(0xF6));  /* disk_fill */
> > +    aml_append(fdi, aml_int(0x0F));  /* disk_head_sttl */
> > +    aml_append(fdi, aml_int(0x08));  /* disk_motor_strt */
> > +
> > +    aml_append(dev, aml_name_decl("_FDI", fdi));
> > +    return dev;
> > +}
> > +
> > +static void fdc_isa_build_aml(ISADevice *isadev, Aml *scope)
> > +{
> > +    Aml *dev;
> > +    Aml *crs;
> > +    int i;
> > +
> > +#define ACPI_FDE_MAX_FD 4
> > +    uint32_t fde_buf[5] = {
> > +        0, 0, 0, 0,     /* presence of floppy drives #0 - #3 */
> > +        cpu_to_le32(2)  /* tape presence (2 == never present) */
> > +    };
> > +
> > +    crs = aml_resource_template();
> > +    aml_append(crs, aml_io(AML_DECODE16, 0x03F2, 0x03F2, 0x00, 0x04));
> > +    aml_append(crs, aml_io(AML_DECODE16, 0x03F7, 0x03F7, 0x00, 0x01));
> > +    aml_append(crs, aml_irq_no_flags(6));
> > +    aml_append(crs,
> > +        aml_dma(AML_COMPATIBILITY, AML_NOTBUSMASTER, AML_TRANSFER8, 2));
> > +
> > +    dev = aml_device("FDC0");
> > +    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0700")));
> > +    aml_append(dev, aml_name_decl("_CRS", crs));
> > +
> > +    for (i = 0; i < MIN(MAX_FD, ACPI_FDE_MAX_FD); i++) {
> > +        FloppyDriveType type = isa_fdc_get_drive_type(isadev, i);
> > +
> > +        if (type < FLOPPY_DRIVE_TYPE_NONE) {
> > +            fde_buf[i] = cpu_to_le32(1);  /* drive present */
> > +            aml_append(dev, build_fdinfo_aml(i, type));
> > +        }
> > +    }
> > +    aml_append(dev, aml_name_decl("_FDE",
> > +               aml_buffer(sizeof(fde_buf), (uint8_t *)fde_buf)));
> > +
> > +    aml_append(scope, dev);
> > +}
> > +
> >   static const VMStateDescription vmstate_isa_fdc ={
> >       .name = "fdc",
> >       .version_id = 2,
> > @@ -2798,11 +2879,13 @@ static Property isa_fdc_properties[] = {
> >   static void isabus_fdc_class_init(ObjectClass *klass, void *data)
> >   {
> >       DeviceClass *dc = DEVICE_CLASS(klass);
> > +    ISADeviceClass *isa = ISA_DEVICE_CLASS(klass);
> >       dc->realize = isabus_fdc_realize;
> >       dc->fw_name = "fdc";
> >       dc->reset = fdctrl_external_reset_isa;
> >       dc->vmsd = &vmstate_isa_fdc;
> > +    isa->build_aml = fdc_isa_build_aml;
> >       device_class_set_props(dc, isa_fdc_properties);
> >       set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> >   }
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 443db94deb5b..775936e28b9a 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1058,85 +1058,6 @@ static void build_hpet_aml(Aml *table)
> >       aml_append(table, scope);
> >   }
> > -static Aml *build_fdinfo_aml(int idx, FloppyDriveType type)
> > -{
> > -    Aml *dev, *fdi;
> > -    uint8_t maxc, maxh, maxs;
> > -
> > -    isa_fdc_get_drive_max_chs(type, &maxc, &maxh, &maxs);
> > -
> > -    dev = aml_device("FLP%c", 'A' + idx);
> > -
> > -    aml_append(dev, aml_name_decl("_ADR", aml_int(idx)));
> > -
> > -    fdi = aml_package(16);
> > -    aml_append(fdi, aml_int(idx));  /* Drive Number */
> > -    aml_append(fdi,
> > -        aml_int(cmos_get_fd_drive_type(type)));  /* Device Type */
> > -    /*
> > -     * the values below are the limits of the drive, and are thus 
> > independent
> > -     * of the inserted media
> > -     */
> > -    aml_append(fdi, aml_int(maxc));  /* Maximum Cylinder Number */
> > -    aml_append(fdi, aml_int(maxs));  /* Maximum Sector Number */
> > -    aml_append(fdi, aml_int(maxh));  /* Maximum Head Number */
> > -    /*
> > -     * SeaBIOS returns the below values for int 0x13 func 0x08 regardless 
> > of
> > -     * the drive type, so shall we
> > -     */
> > -    aml_append(fdi, aml_int(0xAF));  /* disk_specify_1 */
> > -    aml_append(fdi, aml_int(0x02));  /* disk_specify_2 */
> > -    aml_append(fdi, aml_int(0x25));  /* disk_motor_wait */
> > -    aml_append(fdi, aml_int(0x02));  /* disk_sector_siz */
> > -    aml_append(fdi, aml_int(0x12));  /* disk_eot */
> > -    aml_append(fdi, aml_int(0x1B));  /* disk_rw_gap */
> > -    aml_append(fdi, aml_int(0xFF));  /* disk_dtl */
> > -    aml_append(fdi, aml_int(0x6C));  /* disk_formt_gap */
> > -    aml_append(fdi, aml_int(0xF6));  /* disk_fill */
> > -    aml_append(fdi, aml_int(0x0F));  /* disk_head_sttl */
> > -    aml_append(fdi, aml_int(0x08));  /* disk_motor_strt */
> > -
> > -    aml_append(dev, aml_name_decl("_FDI", fdi));
> > -    return dev;
> > -}
> > -
> > -static Aml *build_fdc_device_aml(ISADevice *fdc)
> > -{
> > -    int i;
> > -    Aml *dev;
> > -    Aml *crs;
> > -
> > -#define ACPI_FDE_MAX_FD 4
> > -    uint32_t fde_buf[5] = {
> > -        0, 0, 0, 0,     /* presence of floppy drives #0 - #3 */
> > -        cpu_to_le32(2)  /* tape presence (2 == never present) */
> > -    };
> > -
> > -    dev = aml_device("FDC0");
> > -    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0700")));
> > -
> > -    crs = aml_resource_template();
> > -    aml_append(crs, aml_io(AML_DECODE16, 0x03F2, 0x03F2, 0x00, 0x04));
> > -    aml_append(crs, aml_io(AML_DECODE16, 0x03F7, 0x03F7, 0x00, 0x01));
> > -    aml_append(crs, aml_irq_no_flags(6));
> > -    aml_append(crs,
> > -        aml_dma(AML_COMPATIBILITY, AML_NOTBUSMASTER, AML_TRANSFER8, 2));
> > -    aml_append(dev, aml_name_decl("_CRS", crs));
> > -
> > -    for (i = 0; i < MIN(MAX_FD, ACPI_FDE_MAX_FD); i++) {
> > -        FloppyDriveType type = isa_fdc_get_drive_type(fdc, i);
> > -
> > -        if (type < FLOPPY_DRIVE_TYPE_NONE) {
> > -            fde_buf[i] = cpu_to_le32(1);  /* drive present */
> > -            aml_append(dev, build_fdinfo_aml(i, type));
> > -        }
> > -    }
> > -    aml_append(dev, aml_name_decl("_FDE",
> > -               aml_buffer(sizeof(fde_buf), (uint8_t *)fde_buf)));
> > -
> > -    return dev;
> > -}
> > -
> >   static Aml *build_kbd_device_aml(void)
> >   {
> >       Aml *dev;
> > @@ -1175,7 +1096,6 @@ static Aml *build_mouse_device_aml(void)
> >   static void build_isa_devices_aml(Aml *table)
> >   {
> > -    ISADevice *fdc = pc_find_fdc0();
> >       bool ambiguous;
> >       Aml *scope = aml_scope("_SB.PCI0.ISA");
> > @@ -1183,9 +1103,6 @@ static void build_isa_devices_aml(Aml *table)
> >       aml_append(scope, build_kbd_device_aml());
> >       aml_append(scope, build_mouse_device_aml());
> > -    if (fdc) {
> > -        aml_append(scope, build_fdc_device_aml(fdc));
> > -    }
> >       if (ambiguous) {
> >           error_report("Multiple ISA busses, unable to define IPMI ACPI 
> > data");
> > diff --git a/stubs/cmos.c b/stubs/cmos.c
> > new file mode 100644
> > index 000000000000..416cbe4055ff
> > --- /dev/null
> > +++ b/stubs/cmos.c
> > @@ -0,0 +1,7 @@
> > +#include "qemu/osdep.h"
> > +#include "hw/i386/pc.h"
> > +
> > +int cmos_get_fd_drive_type(FloppyDriveType fd0)
> > +{
> > +    return 0;
> > +}
> > diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> > index 45be5dc0ed78..3cbe472d1c6c 100644
> > --- a/stubs/Makefile.objs
> > +++ b/stubs/Makefile.objs
> > @@ -3,6 +3,7 @@ stub-obj-y += bdrv-next-monitor-owned.o
> >   stub-obj-y += blk-commit-all.o
> >   stub-obj-y += blockdev-close-all-bdrv-states.o
> >   stub-obj-y += clock-warp.o
> > +stub-obj-y += cmos.o
> >   stub-obj-y += cpu-get-clock.o
> >   stub-obj-y += cpu-get-icount.o
> >   stub-obj-y += dump.o
> > 
>