Message ID | 1435239307-13369-3-git-send-email-lersek@redhat.com |
---|---|
State | New |
Headers | show |
Laszlo Ersek <lersek@redhat.com> writes: > With the pc-q35-2.4 machine type, if the user creates an ISA FDC manually: > > -device isa-fdc,driveA=drive-fdc0-0-0 \ > -drive file=...,if=none,id=drive-fdc0-0-0,format=raw > > then the board-default FDC will be skipped, and only the explicitly > requested FDC will exist. qtree-wise, this is correct; however such an FDC > is currently not registered in the CMOS, because that code is only reached > for the board-default FDC. > > The pc_cmos_init_late() one-shot reset handler -- one-shot because the > CMOS is not reprogrammed during warm reset -- should search for any ISA > FDC devices, created implicitly (by board code) or explicitly, and set the > CMOS accordingly to the ISA FDC(s) with iobase=0x3f0: > > - if there is no such FDC, report both drives absent, > - if there is exactly one such FDC, report its drives in the CMOS, > - if there are more than one such FDCs, then pick one (it is not specified > which one), and print a warning about the ambiguity. > > Cc: Jan Tomko <jtomko@redhat.com> > Cc: John Snow <jsnow@redhat.com> > Cc: Markus Armbruster <armbru@redhat.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Reported-by: Jan Tomko <jtomko@redhat.com> > Suggested-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > Reviewed-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
On 06/26/15 11:31, Markus Armbruster wrote: > Laszlo Ersek <lersek@redhat.com> writes: > >> With the pc-q35-2.4 machine type, if the user creates an ISA FDC manually: >> >> -device isa-fdc,driveA=drive-fdc0-0-0 \ >> -drive file=...,if=none,id=drive-fdc0-0-0,format=raw >> >> then the board-default FDC will be skipped, and only the explicitly >> requested FDC will exist. qtree-wise, this is correct; however such an FDC >> is currently not registered in the CMOS, because that code is only reached >> for the board-default FDC. >> >> The pc_cmos_init_late() one-shot reset handler -- one-shot because the >> CMOS is not reprogrammed during warm reset -- should search for any ISA >> FDC devices, created implicitly (by board code) or explicitly, and set the >> CMOS accordingly to the ISA FDC(s) with iobase=0x3f0: >> >> - if there is no such FDC, report both drives absent, >> - if there is exactly one such FDC, report its drives in the CMOS, >> - if there are more than one such FDCs, then pick one (it is not specified >> which one), and print a warning about the ambiguity. >> >> Cc: Jan Tomko <jtomko@redhat.com> >> Cc: John Snow <jsnow@redhat.com> >> Cc: Markus Armbruster <armbru@redhat.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Reported-by: Jan Tomko <jtomko@redhat.com> >> Suggested-by: Markus Armbruster <armbru@redhat.com> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> Reviewed-by: John Snow <jsnow@redhat.com> > > Reviewed-by: Markus Armbruster <armbru@redhat.com> Thank you. Can you or John please send a PULL req for this? (Or include it in an upcoming PULL of yours.) I've been Cc'ing Paolo because the get-maintainer script reported him at the top for the patch set, but I believe he might not have time for this now. Thanks! Laszlo
On 06/26/2015 08:25 AM, Laszlo Ersek wrote: > On 06/26/15 11:31, Markus Armbruster wrote: >> Laszlo Ersek <lersek@redhat.com> writes: >> >>> With the pc-q35-2.4 machine type, if the user creates an ISA FDC manually: >>> >>> -device isa-fdc,driveA=drive-fdc0-0-0 \ >>> -drive file=...,if=none,id=drive-fdc0-0-0,format=raw >>> >>> then the board-default FDC will be skipped, and only the explicitly >>> requested FDC will exist. qtree-wise, this is correct; however such an FDC >>> is currently not registered in the CMOS, because that code is only reached >>> for the board-default FDC. >>> >>> The pc_cmos_init_late() one-shot reset handler -- one-shot because the >>> CMOS is not reprogrammed during warm reset -- should search for any ISA >>> FDC devices, created implicitly (by board code) or explicitly, and set the >>> CMOS accordingly to the ISA FDC(s) with iobase=0x3f0: >>> >>> - if there is no such FDC, report both drives absent, >>> - if there is exactly one such FDC, report its drives in the CMOS, >>> - if there are more than one such FDCs, then pick one (it is not specified >>> which one), and print a warning about the ambiguity. >>> >>> Cc: Jan Tomko <jtomko@redhat.com> >>> Cc: John Snow <jsnow@redhat.com> >>> Cc: Markus Armbruster <armbru@redhat.com> >>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>> Reported-by: Jan Tomko <jtomko@redhat.com> >>> Suggested-by: Markus Armbruster <armbru@redhat.com> >>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >>> Reviewed-by: John Snow <jsnow@redhat.com> >> >> Reviewed-by: Markus Armbruster <armbru@redhat.com> > > Thank you. Can you or John please send a PULL req for this? (Or include > it in an upcoming PULL of yours.) > > I've been Cc'ing Paolo because the get-maintainer script reported him at > the top for the patch set, but I believe he might not have time for this > now. > > Thanks! > Laszlo > This is technically out-of-tree for me, because it's touching init instead of my device. Best guess is Eduardo Habkost, whom I have CC'd.
On Fri, Jun 26, 2015 at 02:50:04PM -0400, John Snow wrote: > On 06/26/2015 08:25 AM, Laszlo Ersek wrote: > > On 06/26/15 11:31, Markus Armbruster wrote: > >> Laszlo Ersek <lersek@redhat.com> writes: > >> > >>> With the pc-q35-2.4 machine type, if the user creates an ISA FDC manually: > >>> > >>> -device isa-fdc,driveA=drive-fdc0-0-0 \ > >>> -drive file=...,if=none,id=drive-fdc0-0-0,format=raw > >>> > >>> then the board-default FDC will be skipped, and only the explicitly > >>> requested FDC will exist. qtree-wise, this is correct; however such an FDC > >>> is currently not registered in the CMOS, because that code is only reached > >>> for the board-default FDC. > >>> > >>> The pc_cmos_init_late() one-shot reset handler -- one-shot because the > >>> CMOS is not reprogrammed during warm reset -- should search for any ISA > >>> FDC devices, created implicitly (by board code) or explicitly, and set the > >>> CMOS accordingly to the ISA FDC(s) with iobase=0x3f0: > >>> > >>> - if there is no such FDC, report both drives absent, > >>> - if there is exactly one such FDC, report its drives in the CMOS, > >>> - if there are more than one such FDCs, then pick one (it is not specified > >>> which one), and print a warning about the ambiguity. > >>> > >>> Cc: Jan Tomko <jtomko@redhat.com> > >>> Cc: John Snow <jsnow@redhat.com> > >>> Cc: Markus Armbruster <armbru@redhat.com> > >>> Cc: Paolo Bonzini <pbonzini@redhat.com> > >>> Reported-by: Jan Tomko <jtomko@redhat.com> > >>> Suggested-by: Markus Armbruster <armbru@redhat.com> > >>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> > >>> Reviewed-by: John Snow <jsnow@redhat.com> > >> > >> Reviewed-by: Markus Armbruster <armbru@redhat.com> > > > > Thank you. Can you or John please send a PULL req for this? (Or include > > it in an upcoming PULL of yours.) > > > > I've been Cc'ing Paolo because the get-maintainer script reported him at > > the top for the patch set, but I believe he might not have time for this > > now. > > > > Thanks! > > Laszlo > > > > This is technically out-of-tree for me, because it's touching init > instead of my device. > > Best guess is Eduardo Habkost, whom I have CC'd. Michael is the PC maintainer.
Eduardo Habkost <ehabkost@redhat.com> writes: > On Fri, Jun 26, 2015 at 02:50:04PM -0400, John Snow wrote: >> On 06/26/2015 08:25 AM, Laszlo Ersek wrote: >> > On 06/26/15 11:31, Markus Armbruster wrote: >> >> Laszlo Ersek <lersek@redhat.com> writes: >> >> >> >>> With the pc-q35-2.4 machine type, if the user creates an ISA FDC manually: >> >>> >> >>> -device isa-fdc,driveA=drive-fdc0-0-0 \ >> >>> -drive file=...,if=none,id=drive-fdc0-0-0,format=raw >> >>> >> >>> then the board-default FDC will be skipped, and only the explicitly >> >>> requested FDC will exist. qtree-wise, this is correct; however such an FDC >> >>> is currently not registered in the CMOS, because that code is only reached >> >>> for the board-default FDC. >> >>> >> >>> The pc_cmos_init_late() one-shot reset handler -- one-shot because the >> >>> CMOS is not reprogrammed during warm reset -- should search for any ISA >> >>> FDC devices, created implicitly (by board code) or explicitly, and set the >> >>> CMOS accordingly to the ISA FDC(s) with iobase=0x3f0: >> >>> >> >>> - if there is no such FDC, report both drives absent, >> >>> - if there is exactly one such FDC, report its drives in the CMOS, >> >>> - if there are more than one such FDCs, then pick one (it is not specified >> >>> which one), and print a warning about the ambiguity. >> >>> >> >>> Cc: Jan Tomko <jtomko@redhat.com> >> >>> Cc: John Snow <jsnow@redhat.com> >> >>> Cc: Markus Armbruster <armbru@redhat.com> >> >>> Cc: Paolo Bonzini <pbonzini@redhat.com> >> >>> Reported-by: Jan Tomko <jtomko@redhat.com> >> >>> Suggested-by: Markus Armbruster <armbru@redhat.com> >> >>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> >>> Reviewed-by: John Snow <jsnow@redhat.com> >> >> >> >> Reviewed-by: Markus Armbruster <armbru@redhat.com> >> > >> > Thank you. Can you or John please send a PULL req for this? (Or include >> > it in an upcoming PULL of yours.) >> > >> > I've been Cc'ing Paolo because the get-maintainer script reported him at >> > the top for the patch set, but I believe he might not have time for this >> > now. >> > >> > Thanks! >> > Laszlo >> > >> >> This is technically out-of-tree for me, because it's touching init >> instead of my device. >> >> Best guess is Eduardo Habkost, whom I have CC'd. > > Michael is the PC maintainer. Michael, we really needs this series in 2.4, because without it floppy is broken for Q35.
On Fri, Jun 26, 2015 at 02:25:42PM +0200, Laszlo Ersek wrote: > On 06/26/15 11:31, Markus Armbruster wrote: > > Laszlo Ersek <lersek@redhat.com> writes: > > > >> With the pc-q35-2.4 machine type, if the user creates an ISA FDC manually: > >> > >> -device isa-fdc,driveA=drive-fdc0-0-0 \ > >> -drive file=...,if=none,id=drive-fdc0-0-0,format=raw > >> > >> then the board-default FDC will be skipped, and only the explicitly > >> requested FDC will exist. qtree-wise, this is correct; however such an FDC > >> is currently not registered in the CMOS, because that code is only reached > >> for the board-default FDC. > >> > >> The pc_cmos_init_late() one-shot reset handler -- one-shot because the > >> CMOS is not reprogrammed during warm reset -- should search for any ISA > >> FDC devices, created implicitly (by board code) or explicitly, and set the > >> CMOS accordingly to the ISA FDC(s) with iobase=0x3f0: > >> > >> - if there is no such FDC, report both drives absent, > >> - if there is exactly one such FDC, report its drives in the CMOS, > >> - if there are more than one such FDCs, then pick one (it is not specified > >> which one), and print a warning about the ambiguity. > >> > >> Cc: Jan Tomko <jtomko@redhat.com> > >> Cc: John Snow <jsnow@redhat.com> > >> Cc: Markus Armbruster <armbru@redhat.com> > >> Cc: Paolo Bonzini <pbonzini@redhat.com> > >> Reported-by: Jan Tomko <jtomko@redhat.com> > >> Suggested-by: Markus Armbruster <armbru@redhat.com> > >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> > >> Reviewed-by: John Snow <jsnow@redhat.com> > > > > Reviewed-by: Markus Armbruster <armbru@redhat.com> > > Thank you. Can you or John please send a PULL req for this? (Or include > it in an upcoming PULL of yours.) > > I've been Cc'ing Paolo because the get-maintainer script reported him at > the top for the patch set, but I believe he might not have time for this > now. > > Thanks! > Laszlo It's listed as part of PC in MAINTAINERS, but looks like get-maintainer didn't recognize the format. I fixed it up, will review and queue the patches.
On Mon, Jun 29, 2015 at 11:33:42AM +0200, Markus Armbruster wrote: > Eduardo Habkost <ehabkost@redhat.com> writes: > > > On Fri, Jun 26, 2015 at 02:50:04PM -0400, John Snow wrote: > >> On 06/26/2015 08:25 AM, Laszlo Ersek wrote: > >> > On 06/26/15 11:31, Markus Armbruster wrote: > >> >> Laszlo Ersek <lersek@redhat.com> writes: > >> >> > >> >>> With the pc-q35-2.4 machine type, if the user creates an ISA FDC manually: > >> >>> > >> >>> -device isa-fdc,driveA=drive-fdc0-0-0 \ > >> >>> -drive file=...,if=none,id=drive-fdc0-0-0,format=raw > >> >>> > >> >>> then the board-default FDC will be skipped, and only the explicitly > >> >>> requested FDC will exist. qtree-wise, this is correct; however such an FDC > >> >>> is currently not registered in the CMOS, because that code is only reached > >> >>> for the board-default FDC. > >> >>> > >> >>> The pc_cmos_init_late() one-shot reset handler -- one-shot because the > >> >>> CMOS is not reprogrammed during warm reset -- should search for any ISA > >> >>> FDC devices, created implicitly (by board code) or explicitly, and set the > >> >>> CMOS accordingly to the ISA FDC(s) with iobase=0x3f0: > >> >>> > >> >>> - if there is no such FDC, report both drives absent, > >> >>> - if there is exactly one such FDC, report its drives in the CMOS, > >> >>> - if there are more than one such FDCs, then pick one (it is not specified > >> >>> which one), and print a warning about the ambiguity. > >> >>> > >> >>> Cc: Jan Tomko <jtomko@redhat.com> > >> >>> Cc: John Snow <jsnow@redhat.com> > >> >>> Cc: Markus Armbruster <armbru@redhat.com> > >> >>> Cc: Paolo Bonzini <pbonzini@redhat.com> > >> >>> Reported-by: Jan Tomko <jtomko@redhat.com> > >> >>> Suggested-by: Markus Armbruster <armbru@redhat.com> > >> >>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> > >> >>> Reviewed-by: John Snow <jsnow@redhat.com> > >> >> > >> >> Reviewed-by: Markus Armbruster <armbru@redhat.com> > >> > > >> > Thank you. Can you or John please send a PULL req for this? (Or include > >> > it in an upcoming PULL of yours.) > >> > > >> > I've been Cc'ing Paolo because the get-maintainer script reported him at > >> > the top for the patch set, but I believe he might not have time for this > >> > now. > >> > > >> > Thanks! > >> > Laszlo > >> > > >> > >> This is technically out-of-tree for me, because it's touching init > >> instead of my device. > >> > >> Best guess is Eduardo Habkost, whom I have CC'd. > > > > Michael is the PC maintainer. > > Michael, we really needs this series in 2.4, because without it floppy > is broken for Q35. Will review, thanks for the reminder.
On 06/29/15 11:56, Michael S. Tsirkin wrote: > On Mon, Jun 29, 2015 at 11:33:42AM +0200, Markus Armbruster wrote: >> Eduardo Habkost <ehabkost@redhat.com> writes: >> >>> On Fri, Jun 26, 2015 at 02:50:04PM -0400, John Snow wrote: >>>> On 06/26/2015 08:25 AM, Laszlo Ersek wrote: >>>>> On 06/26/15 11:31, Markus Armbruster wrote: >>>>>> Laszlo Ersek <lersek@redhat.com> writes: >>>>>> >>>>>>> With the pc-q35-2.4 machine type, if the user creates an ISA FDC manually: >>>>>>> >>>>>>> -device isa-fdc,driveA=drive-fdc0-0-0 \ >>>>>>> -drive file=...,if=none,id=drive-fdc0-0-0,format=raw >>>>>>> >>>>>>> then the board-default FDC will be skipped, and only the explicitly >>>>>>> requested FDC will exist. qtree-wise, this is correct; however such an FDC >>>>>>> is currently not registered in the CMOS, because that code is only reached >>>>>>> for the board-default FDC. >>>>>>> >>>>>>> The pc_cmos_init_late() one-shot reset handler -- one-shot because the >>>>>>> CMOS is not reprogrammed during warm reset -- should search for any ISA >>>>>>> FDC devices, created implicitly (by board code) or explicitly, and set the >>>>>>> CMOS accordingly to the ISA FDC(s) with iobase=0x3f0: >>>>>>> >>>>>>> - if there is no such FDC, report both drives absent, >>>>>>> - if there is exactly one such FDC, report its drives in the CMOS, >>>>>>> - if there are more than one such FDCs, then pick one (it is not specified >>>>>>> which one), and print a warning about the ambiguity. >>>>>>> >>>>>>> Cc: Jan Tomko <jtomko@redhat.com> >>>>>>> Cc: John Snow <jsnow@redhat.com> >>>>>>> Cc: Markus Armbruster <armbru@redhat.com> >>>>>>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>>>>>> Reported-by: Jan Tomko <jtomko@redhat.com> >>>>>>> Suggested-by: Markus Armbruster <armbru@redhat.com> >>>>>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >>>>>>> Reviewed-by: John Snow <jsnow@redhat.com> >>>>>> >>>>>> Reviewed-by: Markus Armbruster <armbru@redhat.com> >>>>> >>>>> Thank you. Can you or John please send a PULL req for this? (Or include >>>>> it in an upcoming PULL of yours.) >>>>> >>>>> I've been Cc'ing Paolo because the get-maintainer script reported him at >>>>> the top for the patch set, but I believe he might not have time for this >>>>> now. >>>>> >>>>> Thanks! >>>>> Laszlo >>>>> >>>> >>>> This is technically out-of-tree for me, because it's touching init >>>> instead of my device. >>>> >>>> Best guess is Eduardo Habkost, whom I have CC'd. >>> >>> Michael is the PC maintainer. >> >> Michael, we really needs this series in 2.4, because without it floppy >> is broken for Q35. > > Will review, thanks for the reminder. Ping. :) This should go into 2.4, preferably. (Tomorrow ^W in two minutes in my timezone is the rc0 / hard freeze date (according to <http://wiki.qemu.org/Planning/2.4>), but the series is a bugfix.) Thanks Laszlo
diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 4a835be..23616ae 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -335,6 +335,41 @@ typedef struct pc_cmos_init_late_arg { BusState *idebus[2]; } pc_cmos_init_late_arg; +typedef struct check_fdc_state { + ISADevice *floppy; + bool multiple; +} CheckFdcState; + +static int check_fdc(Object *obj, void *opaque) +{ + CheckFdcState *state = opaque; + Object *fdc; + uint32_t iobase; + Error *local_err = NULL; + + fdc = object_dynamic_cast(obj, TYPE_ISA_FDC); + if (!fdc) { + return 0; + } + + iobase = object_property_get_int(obj, "iobase", &local_err); + if (local_err || iobase != 0x3f0) { + error_free(local_err); + return 0; + } + + if (state->floppy) { + state->multiple = true; + } else { + state->floppy = ISA_DEVICE(obj); + } + return 0; +} + +static const char * const fdc_container_path[] = { + "/unattached", "/peripheral", "/peripheral-anon" +}; + static void pc_cmos_init_late(void *opaque) { pc_cmos_init_late_arg *arg = opaque; @@ -343,6 +378,8 @@ static void pc_cmos_init_late(void *opaque) int8_t heads, sectors; int val; int i, trans; + Object *container; + CheckFdcState state = { 0 }; val = 0; if (ide_get_geometry(arg->idebus[0], 0, @@ -372,6 +409,23 @@ static void pc_cmos_init_late(void *opaque) } rtc_set_memory(s, 0x39, val); + /* + * Locate the FDC at IO address 0x3f0, and configure the CMOS registers + * accordingly. + */ + for (i = 0; i < ARRAY_SIZE(fdc_container_path); i++) { + container = container_get(qdev_get_machine(), fdc_container_path[i]); + object_child_foreach(container, check_fdc, &state); + } + + if (state.multiple) { + error_report("warning: multiple floppy disk controllers with " + "iobase=0x3f0 have been found;\n" + "the one being picked for CMOS setup might not reflect " + "your intent"); + } + pc_cmos_init_floppy(s, state.floppy); + qemu_unregister_reset(pc_cmos_init_late, opaque); } @@ -441,9 +495,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size, val |= 0x02; /* FPU is there */ val |= 0x04; /* PS/2 mouse installed */ rtc_set_memory(s, REG_EQUIPMENT_BYTE, val); - pc_cmos_init_floppy(s, floppy); - /* hard drives */ + /* hard drives and FDC */ arg.rtc_state = s; arg.idebus[0] = idebus0; arg.idebus[1] = idebus1;