| Message ID | 6b7af110e1576d6873777673b3e3fa01f4c9839e.1583867210.git.balaton@eik.bme.hu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Implement "non 100% native mode" in via-ide | expand |
On 10/03/2020 19:06, BALATON Zoltan wrote: > Some machines operate in "non 100% native mode" where interrupts are > fixed at legacy IDE interrupts and some guests expect this behaviour > without checking based on knowledge about hardware. Even Linux has > arch specific workarounds for this that are activated on such boards > so this needs to be emulated as well. > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > > Notes: > v2: Don't use PCI_INTERRUPT_LINE in via_ide_set_irq() > v3: Patch pci.c instead of local workaround for PCI reset clearing > PCI_INTERRUPT_PIN config reg > > hw/ide/via.c | 30 ++++++++++++++++++++++++------ > 1 file changed, 24 insertions(+), 6 deletions(-) > > diff --git a/hw/ide/via.c b/hw/ide/via.c > index df0b352b58..1eada23097 100644 > --- a/hw/ide/via.c > +++ b/hw/ide/via.c > @@ -1,9 +1,10 @@ > /* > - * QEMU IDE Emulation: PCI VIA82C686B support. > + * QEMU VIA southbridge IDE emulation (VT82C686B, VT8231) > * > * Copyright (c) 2003 Fabrice Bellard > * Copyright (c) 2006 Openedhand Ltd. > * Copyright (c) 2010 Huacai Chen <zltjiangshi@gmail.com> > + * Copyright (c) 2019-2020 BALATON Zoltan > * > * Permission is hereby granted, free of charge, to any person obtaining a copy > * of this software and associated documentation files (the "Software"), to deal > @@ -25,6 +26,7 @@ > */ > > #include "qemu/osdep.h" > +#include "hw/qdev-properties.h" > #include "hw/pci/pci.h" > #include "migration/vmstate.h" > #include "qemu/module.h" > @@ -111,11 +113,18 @@ static void via_ide_set_irq(void *opaque, int n, int level) > } else { > d->config[0x70 + n * 8] &= ~0x80; > } > - > level = (d->config[0x70] & 0x80) || (d->config[0x78] & 0x80); > - n = pci_get_byte(d->config + PCI_INTERRUPT_LINE); > - if (n) { > - qemu_set_irq(isa_get_irq(NULL, n), level); > + > + /* > + * Some machines operate in "non 100% native mode" where PCI_INTERRUPT_LINE > + * is not used but IDE always uses ISA IRQ 14 and 15 even in native mode. > + * Some guest drivers expect this, often without checking. > + */ > + if (!(pci_get_byte(d->config + PCI_CLASS_PROG) & (n ? 4 : 1)) || > + PCI_IDE(d)->flags & BIT(PCI_IDE_LEGACY_IRQ)) { > + qemu_set_irq(isa_get_irq(NULL, (n ? 15 : 14)), level); > + } else { > + qemu_set_irq(isa_get_irq(NULL, 14), level); > } > } > > @@ -169,7 +178,9 @@ static void via_ide_realize(PCIDevice *dev, Error **errp) > > pci_config_set_prog_interface(pci_conf, 0x8f); /* native PCI ATA mode */ > pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x000000c0); > - dev->wmask[PCI_INTERRUPT_LINE] = 0xf; > + /* Bits 0 and 4 of CLASS_PROG select native mode and are writable */ > + dev->wmask[PCI_CLASS_PROG] = 5; > + dev->wmask[PCI_INTERRUPT_LINE] = 0; > > memory_region_init_io(&d->data_bar[0], OBJECT(d), &pci_ide_data_le_ops, > &d->bus[0], "via-ide0-data", 8); > @@ -213,6 +224,12 @@ static void via_ide_exitfn(PCIDevice *dev) > } > } > > +static Property via_ide_properties[] = { > + DEFINE_PROP_BIT("legacy-irq", PCIIDEState, flags, PCI_IDE_LEGACY_IRQ, > + false), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static void via_ide_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > @@ -225,6 +242,7 @@ static void via_ide_class_init(ObjectClass *klass, void *data) > k->device_id = PCI_DEVICE_ID_VIA_IDE; > k->revision = 0x06; > k->class_id = PCI_CLASS_STORAGE_IDE; > + device_class_set_props(dc, via_ide_properties); > set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > } I don't think this (and adding the feature bit) are the right solution here. I'll do my best to look at the test cases you've sent off-list over the next couple of days and report back. ATB, Mark.
On Wed, 11 Mar 2020, Mark Cave-Ayland wrote: > On 10/03/2020 19:06, BALATON Zoltan wrote: >> Some machines operate in "non 100% native mode" where interrupts are >> fixed at legacy IDE interrupts and some guests expect this behaviour >> without checking based on knowledge about hardware. Even Linux has >> arch specific workarounds for this that are activated on such boards >> so this needs to be emulated as well. >> >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >> --- >> >> Notes: >> v2: Don't use PCI_INTERRUPT_LINE in via_ide_set_irq() >> v3: Patch pci.c instead of local workaround for PCI reset clearing >> PCI_INTERRUPT_PIN config reg >> >> hw/ide/via.c | 30 ++++++++++++++++++++++++------ >> 1 file changed, 24 insertions(+), 6 deletions(-) >> >> diff --git a/hw/ide/via.c b/hw/ide/via.c >> index df0b352b58..1eada23097 100644 >> --- a/hw/ide/via.c >> +++ b/hw/ide/via.c >> @@ -1,9 +1,10 @@ >> /* >> - * QEMU IDE Emulation: PCI VIA82C686B support. >> + * QEMU VIA southbridge IDE emulation (VT82C686B, VT8231) >> * >> * Copyright (c) 2003 Fabrice Bellard >> * Copyright (c) 2006 Openedhand Ltd. >> * Copyright (c) 2010 Huacai Chen <zltjiangshi@gmail.com> >> + * Copyright (c) 2019-2020 BALATON Zoltan >> * >> * Permission is hereby granted, free of charge, to any person obtaining a copy >> * of this software and associated documentation files (the "Software"), to deal >> @@ -25,6 +26,7 @@ >> */ >> >> #include "qemu/osdep.h" >> +#include "hw/qdev-properties.h" >> #include "hw/pci/pci.h" >> #include "migration/vmstate.h" >> #include "qemu/module.h" >> @@ -111,11 +113,18 @@ static void via_ide_set_irq(void *opaque, int n, int level) >> } else { >> d->config[0x70 + n * 8] &= ~0x80; >> } >> - >> level = (d->config[0x70] & 0x80) || (d->config[0x78] & 0x80); >> - n = pci_get_byte(d->config + PCI_INTERRUPT_LINE); >> - if (n) { >> - qemu_set_irq(isa_get_irq(NULL, n), level); >> + >> + /* >> + * Some machines operate in "non 100% native mode" where PCI_INTERRUPT_LINE >> + * is not used but IDE always uses ISA IRQ 14 and 15 even in native mode. >> + * Some guest drivers expect this, often without checking. >> + */ >> + if (!(pci_get_byte(d->config + PCI_CLASS_PROG) & (n ? 4 : 1)) || >> + PCI_IDE(d)->flags & BIT(PCI_IDE_LEGACY_IRQ)) { >> + qemu_set_irq(isa_get_irq(NULL, (n ? 15 : 14)), level); >> + } else { >> + qemu_set_irq(isa_get_irq(NULL, 14), level); >> } >> } >> >> @@ -169,7 +178,9 @@ static void via_ide_realize(PCIDevice *dev, Error **errp) >> >> pci_config_set_prog_interface(pci_conf, 0x8f); /* native PCI ATA mode */ >> pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x000000c0); >> - dev->wmask[PCI_INTERRUPT_LINE] = 0xf; >> + /* Bits 0 and 4 of CLASS_PROG select native mode and are writable */ >> + dev->wmask[PCI_CLASS_PROG] = 5; >> + dev->wmask[PCI_INTERRUPT_LINE] = 0; >> >> memory_region_init_io(&d->data_bar[0], OBJECT(d), &pci_ide_data_le_ops, >> &d->bus[0], "via-ide0-data", 8); >> @@ -213,6 +224,12 @@ static void via_ide_exitfn(PCIDevice *dev) >> } >> } >> >> +static Property via_ide_properties[] = { >> + DEFINE_PROP_BIT("legacy-irq", PCIIDEState, flags, PCI_IDE_LEGACY_IRQ, >> + false), >> + DEFINE_PROP_END_OF_LIST(), >> +}; >> + >> static void via_ide_class_init(ObjectClass *klass, void *data) >> { >> DeviceClass *dc = DEVICE_CLASS(klass); >> @@ -225,6 +242,7 @@ static void via_ide_class_init(ObjectClass *klass, void *data) >> k->device_id = PCI_DEVICE_ID_VIA_IDE; >> k->revision = 0x06; >> k->class_id = PCI_CLASS_STORAGE_IDE; >> + device_class_set_props(dc, via_ide_properties); >> set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); >> } > > I don't think this (and adding the feature bit) are the right solution here. I'll do > my best to look at the test cases you've sent off-list over the next couple of days > and report back. Thanks. No need to hurry as I don't have much free time for this now so maybe I'll come back to it when submitting pegasos2 patches. Until then if you can take at least the reviewed patches then we can revise the remaining ones when cleaning up VT82C686B and VT8231 models as that might be related to qdevifying this, I don't see a way to avoid some property to tell the device that it should behave differently on pegasos2 than on fulong2e and using a feature bit for this seems to be the least hacky solution to me. Your proposed imaginary gpios seem more complex and likely not really match hardware so I don't think that would be less of a hack. I've also tried using PCI_INTERRUPT_PIN config reg which at least the 686B datasheet said would switch to legacy IRQ routing but this did not work with Linux and MorphOS on pegasos2. We could also abuse another unused config reg value for this that clients don't care about but that wouldn't be any cleaner either. Regards, BALATON Zoltan
diff --git a/hw/ide/via.c b/hw/ide/via.c index df0b352b58..1eada23097 100644 --- a/hw/ide/via.c +++ b/hw/ide/via.c @@ -1,9 +1,10 @@ /* - * QEMU IDE Emulation: PCI VIA82C686B support. + * QEMU VIA southbridge IDE emulation (VT82C686B, VT8231) * * Copyright (c) 2003 Fabrice Bellard * Copyright (c) 2006 Openedhand Ltd. * Copyright (c) 2010 Huacai Chen <zltjiangshi@gmail.com> + * Copyright (c) 2019-2020 BALATON Zoltan * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -25,6 +26,7 @@ */ #include "qemu/osdep.h" +#include "hw/qdev-properties.h" #include "hw/pci/pci.h" #include "migration/vmstate.h" #include "qemu/module.h" @@ -111,11 +113,18 @@ static void via_ide_set_irq(void *opaque, int n, int level) } else { d->config[0x70 + n * 8] &= ~0x80; } - level = (d->config[0x70] & 0x80) || (d->config[0x78] & 0x80); - n = pci_get_byte(d->config + PCI_INTERRUPT_LINE); - if (n) { - qemu_set_irq(isa_get_irq(NULL, n), level); + + /* + * Some machines operate in "non 100% native mode" where PCI_INTERRUPT_LINE + * is not used but IDE always uses ISA IRQ 14 and 15 even in native mode. + * Some guest drivers expect this, often without checking. + */ + if (!(pci_get_byte(d->config + PCI_CLASS_PROG) & (n ? 4 : 1)) || + PCI_IDE(d)->flags & BIT(PCI_IDE_LEGACY_IRQ)) { + qemu_set_irq(isa_get_irq(NULL, (n ? 15 : 14)), level); + } else { + qemu_set_irq(isa_get_irq(NULL, 14), level); } } @@ -169,7 +178,9 @@ static void via_ide_realize(PCIDevice *dev, Error **errp) pci_config_set_prog_interface(pci_conf, 0x8f); /* native PCI ATA mode */ pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x000000c0); - dev->wmask[PCI_INTERRUPT_LINE] = 0xf; + /* Bits 0 and 4 of CLASS_PROG select native mode and are writable */ + dev->wmask[PCI_CLASS_PROG] = 5; + dev->wmask[PCI_INTERRUPT_LINE] = 0; memory_region_init_io(&d->data_bar[0], OBJECT(d), &pci_ide_data_le_ops, &d->bus[0], "via-ide0-data", 8); @@ -213,6 +224,12 @@ static void via_ide_exitfn(PCIDevice *dev) } } +static Property via_ide_properties[] = { + DEFINE_PROP_BIT("legacy-irq", PCIIDEState, flags, PCI_IDE_LEGACY_IRQ, + false), + DEFINE_PROP_END_OF_LIST(), +}; + static void via_ide_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -225,6 +242,7 @@ static void via_ide_class_init(ObjectClass *klass, void *data) k->device_id = PCI_DEVICE_ID_VIA_IDE; k->revision = 0x06; k->class_id = PCI_CLASS_STORAGE_IDE; + device_class_set_props(dc, via_ide_properties); set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); }
Some machines operate in "non 100% native mode" where interrupts are fixed at legacy IDE interrupts and some guests expect this behaviour without checking based on knowledge about hardware. Even Linux has arch specific workarounds for this that are activated on such boards so this needs to be emulated as well. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- Notes: v2: Don't use PCI_INTERRUPT_LINE in via_ide_set_irq() v3: Patch pci.c instead of local workaround for PCI reset clearing PCI_INTERRUPT_PIN config reg hw/ide/via.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-)