Re: [PATCH 2/2] via-ide: Fix fuloong2e support
On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote:
On 12/27/20 5:40 PM, BALATON Zoltan via wrote:
On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote:
On 12/25/20 12:23 AM, BALATON Zoltan wrote:
From: Guenter Roeck
Fuloong2e needs to use legacy mode for IDE support to work with Linux.
Add property to via-ide driver to make the mode configurable, and set
legacy mode for Fuloong2e.
Fixes: 4ea98d317eb ("ide/via: Implement and use native PCI IDE mode")?
Not really. That patch did what it said (only emulating (half) native
mode instead of only emulating legacy mode) so it wasn't broken per se
but it turned out that approach wasn't good enough for all use cases so
this now takes a different turn (emulating either legacy or half-native
mode based on option property). Therefore. I don't think Fixes: applies
in this case. It fixes an issue with a guest but replaces previous patch
with different approach. (Even though it reuses most of it.)
Well, if Linux guest got broken by this commit, why not name it a "fix"?
We do call it a fix in the patch title. I just thought Fixes: tag was more
for either security fixes or cases when original commit had a bug that's
fixed up by this patch which is not exactly the case here.
Anyway I don't mind how it is called. I find important to refer to the
commit hash to help navigating between commits while reviewing history.
What about:
'''
The legacy mode for IDE support has been removed in commit 4ea98d317eb
("ide/via: Implement and use native PCI IDE mode"). When using a Linux
guest, the Fuloong2e machine requires the legacy mode.
Add property to via-ide driver to make the mode configurable, and set
legacy mode for Fuloong2e.
'''
Guenter, is that OK with you? (I can update when applying this series
via the MIPS tree).
I've submitted v2 with this commit message (slightly edited) mentioning
the original commit for tracking.
Regards,
BALATON Zoltan
Re: [PATCH 2/2] via-ide: Fix fuloong2e support
On 12/27/20 9:24 AM, Philippe Mathieu-Daudé wrote:
> On 12/27/20 5:40 PM, BALATON Zoltan via wrote:
>> On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote:
>>> On 12/25/20 12:23 AM, BALATON Zoltan wrote:
From: Guenter Roeck
Fuloong2e needs to use legacy mode for IDE support to work with Linux.
Add property to via-ide driver to make the mode configurable, and set
legacy mode for Fuloong2e.
>>>
>>> Fixes: 4ea98d317eb ("ide/via: Implement and use native PCI IDE mode")?
>>
>> Not really. That patch did what it said (only emulating (half) native
>> mode instead of only emulating legacy mode) so it wasn't broken per se
>> but it turned out that approach wasn't good enough for all use cases so
>> this now takes a different turn (emulating either legacy or half-native
>> mode based on option property). Therefore. I don't think Fixes: applies
>> in this case. It fixes an issue with a guest but replaces previous patch
>> with different approach. (Even though it reuses most of it.)
>
> Well, if Linux guest got broken by this commit, why not name it a "fix"?
> Anyway I don't mind how it is called. I find important to refer to the
> commit hash to help navigating between commits while reviewing history.
>
> What about:
>
> '''
> The legacy mode for IDE support has been removed in commit 4ea98d317eb
> ("ide/via: Implement and use native PCI IDE mode"). When using a Linux
> guest, the Fuloong2e machine requires the legacy mode.
> Add property to via-ide driver to make the mode configurable, and set
> legacy mode for Fuloong2e.
> '''
>
> Guenter, is that OK with you? (I can update when applying this series
> via the MIPS tree).
>
Sure, I don't really care about the commit message as long as the problem
is fixed.
Guenter
Re: [PATCH 2/2] via-ide: Fix fuloong2e support
On 12/27/20 5:40 PM, BALATON Zoltan via wrote:
> On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote:
>> On 12/25/20 12:23 AM, BALATON Zoltan wrote:
>>> From: Guenter Roeck
>>>
>>> Fuloong2e needs to use legacy mode for IDE support to work with Linux.
>>> Add property to via-ide driver to make the mode configurable, and set
>>> legacy mode for Fuloong2e.
>>>
>>
>> Fixes: 4ea98d317eb ("ide/via: Implement and use native PCI IDE mode")?
>
> Not really. That patch did what it said (only emulating (half) native
> mode instead of only emulating legacy mode) so it wasn't broken per se
> but it turned out that approach wasn't good enough for all use cases so
> this now takes a different turn (emulating either legacy or half-native
> mode based on option property). Therefore. I don't think Fixes: applies
> in this case. It fixes an issue with a guest but replaces previous patch
> with different approach. (Even though it reuses most of it.)
Well, if Linux guest got broken by this commit, why not name it a "fix"?
Anyway I don't mind how it is called. I find important to refer to the
commit hash to help navigating between commits while reviewing history.
What about:
'''
The legacy mode for IDE support has been removed in commit 4ea98d317eb
("ide/via: Implement and use native PCI IDE mode"). When using a Linux
guest, the Fuloong2e machine requires the legacy mode.
Add property to via-ide driver to make the mode configurable, and set
legacy mode for Fuloong2e.
'''
Guenter, is that OK with you? (I can update when applying this series
via the MIPS tree).
Thanks,
Phil.
>
> Regards,
> BALATON Zoltan
Re: [PATCH 2/2] via-ide: Fix fuloong2e support
On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote:
On 12/25/20 12:23 AM, BALATON Zoltan wrote:
From: Guenter Roeck
Fuloong2e needs to use legacy mode for IDE support to work with Linux.
Add property to via-ide driver to make the mode configurable, and set
legacy mode for Fuloong2e.
Fixes: 4ea98d317eb ("ide/via: Implement and use native PCI IDE mode")?
Not really. That patch did what it said (only emulating (half) native mode
instead of only emulating legacy mode) so it wasn't broken per se but it
turned out that approach wasn't good enough for all use cases so this now
takes a different turn (emulating either legacy or half-native mode based
on option property). Therefore. I don't think Fixes: applies in this case.
It fixes an issue with a guest but replaces previous patch with different
approach. (Even though it reuses most of it.)
Regards,
BALATON Zoltan
Signed-off-by: Guenter Roeck
[balaton: Use bit in flags for property, add comment for missing BAR4]
Signed-off-by: BALATON Zoltan
---
hw/ide/via.c| 19 +--
hw/mips/fuloong2e.c | 4 +++-
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/hw/ide/via.c b/hw/ide/via.c
index be09912b33..7d54d7e829 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -26,6 +26,7 @@
#include "qemu/osdep.h"
#include "hw/pci/pci.h"
+#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
#include "sysemu/dma.h"
@@ -185,12 +186,19 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
&d->bus[1], "via-ide1-cmd", 4);
pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[1]);
-bmdma_setup_bar(d);
-pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
+if (!(d->flags & BIT(PCI_IDE_LEGACY_MODE))) {
+/* Missing BAR4 will make Linux driver fall back to legacy PIO mode */
+bmdma_setup_bar(d);
+pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
+}
qdev_init_gpio_in(ds, via_ide_set_irq, 2);
for (i = 0; i < 2; i++) {
ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
+if (d->flags & BIT(PCI_IDE_LEGACY_MODE)) {
+ide_init_ioport(&d->bus[i], NULL, i ? 0x170 : 0x1f0,
+i ? 0x376 : 0x3f6);
+}
ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
bmdma_init(&d->bus[i], &d->bmdma[i], d);
@@ -210,6 +218,12 @@ static void via_ide_exitfn(PCIDevice *dev)
}
}
+static Property via_ide_properties[] = {
+DEFINE_PROP_BIT("legacy_mode", PCIIDEState, flags, PCI_IDE_LEGACY_MODE,
+false),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static void via_ide_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -223,6 +237,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);
}
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 45c596f4fe..f0733e87b7 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -253,7 +253,9 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int
slot, qemu_irq intc,
/* Super I/O */
isa_create_simple(isa_bus, TYPE_VT82C686B_SUPERIO);
-dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
+dev = pci_new(PCI_DEVFN(slot, 1), "via-ide");
+qdev_prop_set_bit(&dev->qdev, "legacy_mode", true);
+pci_realize_and_unref(dev, pci_bus, &error_fatal);
pci_ide_create_devs(dev);
pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 2/2] via-ide: Fix fuloong2e support
On 12/25/20 12:23 AM, BALATON Zoltan wrote:
> From: Guenter Roeck
>
> Fuloong2e needs to use legacy mode for IDE support to work with Linux.
> Add property to via-ide driver to make the mode configurable, and set
> legacy mode for Fuloong2e.
>
Fixes: 4ea98d317eb ("ide/via: Implement and use native PCI IDE mode")?
> Signed-off-by: Guenter Roeck
> [balaton: Use bit in flags for property, add comment for missing BAR4]
> Signed-off-by: BALATON Zoltan
> ---
> hw/ide/via.c| 19 +--
> hw/mips/fuloong2e.c | 4 +++-
> 2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index be09912b33..7d54d7e829 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -26,6 +26,7 @@
>
> #include "qemu/osdep.h"
> #include "hw/pci/pci.h"
> +#include "hw/qdev-properties.h"
> #include "migration/vmstate.h"
> #include "qemu/module.h"
> #include "sysemu/dma.h"
> @@ -185,12 +186,19 @@ static void via_ide_realize(PCIDevice *dev, Error
> **errp)
>&d->bus[1], "via-ide1-cmd", 4);
> pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[1]);
>
> -bmdma_setup_bar(d);
> -pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
> +if (!(d->flags & BIT(PCI_IDE_LEGACY_MODE))) {
> +/* Missing BAR4 will make Linux driver fall back to legacy PIO mode
> */
> +bmdma_setup_bar(d);
> +pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
> +}
>
> qdev_init_gpio_in(ds, via_ide_set_irq, 2);
> for (i = 0; i < 2; i++) {
> ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
> +if (d->flags & BIT(PCI_IDE_LEGACY_MODE)) {
> +ide_init_ioport(&d->bus[i], NULL, i ? 0x170 : 0x1f0,
> +i ? 0x376 : 0x3f6);
> +}
> ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
>
> bmdma_init(&d->bus[i], &d->bmdma[i], d);
> @@ -210,6 +218,12 @@ static void via_ide_exitfn(PCIDevice *dev)
> }
> }
>
> +static Property via_ide_properties[] = {
> +DEFINE_PROP_BIT("legacy_mode", PCIIDEState, flags, PCI_IDE_LEGACY_MODE,
> +false),
> +DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void via_ide_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -223,6 +237,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);
> }
>
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index 45c596f4fe..f0733e87b7 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -253,7 +253,9 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus,
> int slot, qemu_irq intc,
> /* Super I/O */
> isa_create_simple(isa_bus, TYPE_VT82C686B_SUPERIO);
>
> -dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
> +dev = pci_new(PCI_DEVFN(slot, 1), "via-ide");
> +qdev_prop_set_bit(&dev->qdev, "legacy_mode", true);
> +pci_realize_and_unref(dev, pci_bus, &error_fatal);
> pci_ide_create_devs(dev);
>
> pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
>
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 2/2] via-ide: Fix fuloong2e support
On Fri, Dec 25, 2020 at 12:23:37AM +0100, BALATON Zoltan wrote:
> From: Guenter Roeck
>
> Fuloong2e needs to use legacy mode for IDE support to work with Linux.
> Add property to via-ide driver to make the mode configurable, and set
> legacy mode for Fuloong2e.
>
> Signed-off-by: Guenter Roeck
> [balaton: Use bit in flags for property, add comment for missing BAR4]
> Signed-off-by: BALATON Zoltan
Tested-by: Guenter Roeck
> ---
> hw/ide/via.c| 19 +--
> hw/mips/fuloong2e.c | 4 +++-
> 2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index be09912b33..7d54d7e829 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -26,6 +26,7 @@
>
> #include "qemu/osdep.h"
> #include "hw/pci/pci.h"
> +#include "hw/qdev-properties.h"
> #include "migration/vmstate.h"
> #include "qemu/module.h"
> #include "sysemu/dma.h"
> @@ -185,12 +186,19 @@ static void via_ide_realize(PCIDevice *dev, Error
> **errp)
>&d->bus[1], "via-ide1-cmd", 4);
> pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[1]);
>
> -bmdma_setup_bar(d);
> -pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
> +if (!(d->flags & BIT(PCI_IDE_LEGACY_MODE))) {
> +/* Missing BAR4 will make Linux driver fall back to legacy PIO mode
> */
> +bmdma_setup_bar(d);
> +pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
> +}
>
> qdev_init_gpio_in(ds, via_ide_set_irq, 2);
> for (i = 0; i < 2; i++) {
> ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
> +if (d->flags & BIT(PCI_IDE_LEGACY_MODE)) {
> +ide_init_ioport(&d->bus[i], NULL, i ? 0x170 : 0x1f0,
> +i ? 0x376 : 0x3f6);
> +}
> ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
>
> bmdma_init(&d->bus[i], &d->bmdma[i], d);
> @@ -210,6 +218,12 @@ static void via_ide_exitfn(PCIDevice *dev)
> }
> }
>
> +static Property via_ide_properties[] = {
> +DEFINE_PROP_BIT("legacy_mode", PCIIDEState, flags, PCI_IDE_LEGACY_MODE,
> +false),
> +DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void via_ide_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -223,6 +237,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);
> }
>
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index 45c596f4fe..f0733e87b7 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -253,7 +253,9 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus,
> int slot, qemu_irq intc,
> /* Super I/O */
> isa_create_simple(isa_bus, TYPE_VT82C686B_SUPERIO);
>
> -dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
> +dev = pci_new(PCI_DEVFN(slot, 1), "via-ide");
> +qdev_prop_set_bit(&dev->qdev, "legacy_mode", true);
> +pci_realize_and_unref(dev, pci_bus, &error_fatal);
> pci_ide_create_devs(dev);
>
> pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
> --
> 2.21.3
>
[PATCH 2/2] via-ide: Fix fuloong2e support
From: Guenter Roeck
Fuloong2e needs to use legacy mode for IDE support to work with Linux.
Add property to via-ide driver to make the mode configurable, and set
legacy mode for Fuloong2e.
Signed-off-by: Guenter Roeck
[balaton: Use bit in flags for property, add comment for missing BAR4]
Signed-off-by: BALATON Zoltan
---
hw/ide/via.c| 19 +--
hw/mips/fuloong2e.c | 4 +++-
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/hw/ide/via.c b/hw/ide/via.c
index be09912b33..7d54d7e829 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -26,6 +26,7 @@
#include "qemu/osdep.h"
#include "hw/pci/pci.h"
+#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
#include "sysemu/dma.h"
@@ -185,12 +186,19 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
&d->bus[1], "via-ide1-cmd", 4);
pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[1]);
-bmdma_setup_bar(d);
-pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
+if (!(d->flags & BIT(PCI_IDE_LEGACY_MODE))) {
+/* Missing BAR4 will make Linux driver fall back to legacy PIO mode */
+bmdma_setup_bar(d);
+pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
+}
qdev_init_gpio_in(ds, via_ide_set_irq, 2);
for (i = 0; i < 2; i++) {
ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
+if (d->flags & BIT(PCI_IDE_LEGACY_MODE)) {
+ide_init_ioport(&d->bus[i], NULL, i ? 0x170 : 0x1f0,
+i ? 0x376 : 0x3f6);
+}
ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
bmdma_init(&d->bus[i], &d->bmdma[i], d);
@@ -210,6 +218,12 @@ static void via_ide_exitfn(PCIDevice *dev)
}
}
+static Property via_ide_properties[] = {
+DEFINE_PROP_BIT("legacy_mode", PCIIDEState, flags, PCI_IDE_LEGACY_MODE,
+false),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static void via_ide_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -223,6 +237,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);
}
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 45c596f4fe..f0733e87b7 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -253,7 +253,9 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int
slot, qemu_irq intc,
/* Super I/O */
isa_create_simple(isa_bus, TYPE_VT82C686B_SUPERIO);
-dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
+dev = pci_new(PCI_DEVFN(slot, 1), "via-ide");
+qdev_prop_set_bit(&dev->qdev, "legacy_mode", true);
+pci_realize_and_unref(dev, pci_bus, &error_fatal);
pci_ide_create_devs(dev);
pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
--
2.21.3
