* [RFC PATCH 1/5] ide: Make room for flags in PCIIDEState and add one for legacy mode
2021-01-01 23:12 [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again Philippe Mathieu-Daudé
@ 2021-01-01 23:12 ` Philippe Mathieu-Daudé
2021-01-01 23:12 ` [RFC PATCH 2/5] via-ide: Fix fuloong2e support Philippe Mathieu-Daudé
` (4 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-01 23:12 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Aleksandar Rikalo, qemu-block, Guenter Roeck,
Huacai Chen, Mark Cave-Ayland, Philippe Mathieu-Daudé,
Artyom Tarasenko, Cleber Rosa, John Snow, Aurelien Jarno,
Wainer dos Santos Moschetta
From: BALATON Zoltan <balaton@eik.bme.hu>
We'll need a flag for implementing some device specific behaviour in
via-ide but we already have a currently CMD646 specific field that can
be repurposed for this and leave room for further flags if needed in
the future. This patch changes the "secondary" field to "flags" and
change CMD646 and its users accordingly and define a new flag for
forcing legacy mode that will be used by via-ide for now.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Message-Id: <28150c05831caea5926d4c6a881f448c2827703d.1609191252.git.balaton@eik.bme.hu>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/ide/pci.h | 7 ++++++-
hw/ide/cmd646.c | 6 +++---
hw/sparc64/sun4u.c | 2 +-
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index d8384e1c422..75d1a32f6d2 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -42,6 +42,11 @@ typedef struct BMDMAState {
#define TYPE_PCI_IDE "pci-ide"
OBJECT_DECLARE_SIMPLE_TYPE(PCIIDEState, PCI_IDE)
+enum {
+ PCI_IDE_SECONDARY, /* used only for cmd646 */
+ PCI_IDE_LEGACY_MODE
+};
+
struct PCIIDEState {
/*< private >*/
PCIDevice parent_obj;
@@ -49,7 +54,7 @@ struct PCIIDEState {
IDEBus bus[2];
BMDMAState bmdma[2];
- uint32_t secondary; /* used only for cmd646 */
+ uint32_t flags;
MemoryRegion bmdma_bar;
MemoryRegion cmd_bar[2];
MemoryRegion data_bar[2];
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index c2546314855..cfea7fca06f 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -255,8 +255,8 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp)
pci_conf[PCI_CLASS_PROG] = 0x8f;
- pci_conf[CNTRL] = CNTRL_EN_CH0; // enable IDE0
- if (d->secondary) {
+ pci_conf[CNTRL] = CNTRL_EN_CH0; /* enable IDE0 */
+ if (d->flags & BIT(PCI_IDE_SECONDARY)) {
/* XXX: if not enabled, really disable the seconday IDE controller */
pci_conf[CNTRL] |= CNTRL_EN_CH1; /* enable IDE1 */
}
@@ -314,7 +314,7 @@ static void pci_cmd646_ide_exitfn(PCIDevice *dev)
}
static Property cmd646_ide_properties[] = {
- DEFINE_PROP_UINT32("secondary", PCIIDEState, secondary, 0),
+ DEFINE_PROP_BIT("secondary", PCIIDEState, flags, PCI_IDE_SECONDARY, false),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 0fa13a73302..c46baa9f489 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -674,7 +674,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
}
pci_dev = pci_new(PCI_DEVFN(3, 0), "cmd646-ide");
- qdev_prop_set_uint32(&pci_dev->qdev, "secondary", 1);
+ qdev_prop_set_bit(&pci_dev->qdev, "secondary", true);
pci_realize_and_unref(pci_dev, pci_busA, &error_fatal);
pci_ide_create_devs(pci_dev);
--
2.26.2
^ permalink raw reply related [flat|nested] 18+ messages in thread* [RFC PATCH 2/5] via-ide: Fix fuloong2e support
2021-01-01 23:12 [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again Philippe Mathieu-Daudé
2021-01-01 23:12 ` [RFC PATCH 1/5] ide: Make room for flags in PCIIDEState and add one for legacy mode Philippe Mathieu-Daudé
@ 2021-01-01 23:12 ` Philippe Mathieu-Daudé
2021-01-03 15:14 ` Mark Cave-Ayland
2021-01-01 23:12 ` [RFC PATCH 3/5] hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified Philippe Mathieu-Daudé
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-01 23:12 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Aleksandar Rikalo, qemu-block, Guenter Roeck,
Huacai Chen, Mark Cave-Ayland, Philippe Mathieu-Daudé,
Artyom Tarasenko, Cleber Rosa, John Snow, Aurelien Jarno,
Wainer dos Santos Moschetta
From: Guenter Roeck <linux@roeck-us.net>
The IDE legacy mode emulation has been removed in commit 4ea98d317eb
("ide/via: Implement and use native PCI IDE mode") but some Linux
kernels (probably including def_config) require legacy mode on the
Fuloong2e so only emulating native mode did not turn out feasible.
Add property to via-ide model to make the mode configurable, and set
legacy mode for Fuloong2e.
[balaton: Use bit in flags for property, add comment for missing BAR4]
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Message-Id: <17a50e58e6baa26440c7dac83f07fdbba1595439.1609191252.git.balaton@eik.bme.hu>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
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 be09912b334..2d935b910f8 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 45c596f4fe5..d334fde389f 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.26.2
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RFC PATCH 2/5] via-ide: Fix fuloong2e support
2021-01-01 23:12 ` [RFC PATCH 2/5] via-ide: Fix fuloong2e support Philippe Mathieu-Daudé
@ 2021-01-03 15:14 ` Mark Cave-Ayland
2021-01-03 18:31 ` BALATON Zoltan via
0 siblings, 1 reply; 18+ messages in thread
From: Mark Cave-Ayland @ 2021-01-03 15:14 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Peter Maydell, Aleksandar Rikalo, qemu-block, Huacai Chen,
Wainer dos Santos Moschetta, Aurelien Jarno, Artyom Tarasenko,
Cleber Rosa, John Snow, Guenter Roeck
On 01/01/2021 23:12, Philippe Mathieu-Daudé wrote:
> From: Guenter Roeck <linux@roeck-us.net>
>
> The IDE legacy mode emulation has been removed in commit 4ea98d317eb
> ("ide/via: Implement and use native PCI IDE mode") but some Linux
> kernels (probably including def_config) require legacy mode on the
> Fuloong2e so only emulating native mode did not turn out feasible.
> Add property to via-ide model to make the mode configurable, and set
> legacy mode for Fuloong2e.
>
> [balaton: Use bit in flags for property, add comment for missing BAR4]
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Tested-by: Guenter Roeck <linux@roeck-us.net>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Message-Id: <17a50e58e6baa26440c7dac83f07fdbba1595439.1609191252.git.balaton@eik.bme.hu>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> 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 be09912b334..2d935b910f8 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));
The if() here needs to be removed because the VIA IDE device starts up in legacy
mode. If you look at the PCI bus master specification [1] the table at the top of
page 3 is the breakdown of the PCI_CLASS_PROG byte: the configured value of 0x8a
clearly indicates both channels are in legacy mode, which means that the PCI BARs are
disabled and the controller is fixed to use the legacy IDE ioports with the option to
switch up to native mode.
This is the main reason that the fuloong2e kernel panics on startup because its reads
PCI_CLASS_PROG, discovers the controller is in legacy mode and then tries to access
the legacy IDE ioports. Since the ioports are no longer mapped then the kernel crashes.
(Fun fact: you can change PCI_CLASS_PROG to 0x8f and watch Linux configure the VIA
IDE controller in native mode: it gets quite far before it decides to eventually panic)
The fix here is simple: always enable the legacy IDE ioports. Zoltan mentioned he was
concerned about having multiple IDE ports available, but this is not an issue for 2
reasons:
1) Before any of Zoltan's patches were applied, the VIA IDE driver set PCI_CLASS_PROG
to 0x8a with the legacy IDE ports always enabled, and there were no issues
2) PCI bus enumeration always leaves the low end of IO space free so that there are
no clashes with legacy ISA devices which can't be freely relocated in IO space
> 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(),
> +};
The other issue with the patch is that the legacy-mode property doesn't have anything
to do with legacy mode: once the legacy IDE ioports are always enabled, the only
thing this property controls is whether PCI BAR 4 is enabled. It doesn't select
legacy mode since this is already the default, and neither does it force it by fixing
the relevant bits in PCI_CLASS_PROG.
PCI BAR 4 is for configuring BMDMA and Linux checks to see if BAR 4 is non-zero: if
it is, then it tries to use BMDMA which is the feature that actually crashes the
fuloong2e Linux kernel. Hence my suggestion to rename the property to
"x-disable-bdma": the x- prefix convention clearly marks this out as an experimental
property whilst also correctly describing what is is used for, just in case it leaks
into external qdev configuration.
> 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 45c596f4fe5..d334fde389f 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");
I hope this helps to summarise all my comments from the previous thread. Given that
it is not possible to set default PCI BAR addresses in QEMU due to the post-qdev PCI
bus reset, the ultimate aim should be to devise a couple of routines pci_enable_bar()
and pci_disable_bar() so that the new property can be removed and controller mode set
directly from configuration space writes to PCI_CLASS_PROG. And as per my reply in
the previous thread, I can't see this happening in the near future.
I would really like to see a version of this patchset merged, particularly the
integration test which will be of clear value moving forward. I am not against hacks
like this where they are necessary, but as part of the review process they need to be
understood with the aim being for QEMU to provide the guest with the most coherent
environment possible.
ATB,
Mark.
[1] PCI IDE Controller: http://www.bswd.com/pciide.pdf
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC PATCH 2/5] via-ide: Fix fuloong2e support
2021-01-03 15:14 ` Mark Cave-Ayland
@ 2021-01-03 18:31 ` BALATON Zoltan via
0 siblings, 0 replies; 18+ messages in thread
From: BALATON Zoltan via @ 2021-01-03 18:31 UTC (permalink / raw)
To: Mark Cave-Ayland
Cc: Peter Maydell, Aleksandar Rikalo, qemu-block, Guenter Roeck,
Huacai Chen, Philippe Mathieu-Daudé,
Wainer dos Santos Moschetta, qemu-devel, Artyom Tarasenko,
Cleber Rosa, John Snow, Aurelien Jarno
[-- Attachment #1: Type: text/plain, Size: 15997 bytes --]
On Sun, 3 Jan 2021, Mark Cave-Ayland wrote:
> On 01/01/2021 23:12, Philippe Mathieu-Daudé wrote:
>> From: Guenter Roeck <linux@roeck-us.net>
>>
>> The IDE legacy mode emulation has been removed in commit 4ea98d317eb
>> ("ide/via: Implement and use native PCI IDE mode") but some Linux
>> kernels (probably including def_config) require legacy mode on the
>> Fuloong2e so only emulating native mode did not turn out feasible.
>> Add property to via-ide model to make the mode configurable, and set
>> legacy mode for Fuloong2e.
>>
>> [balaton: Use bit in flags for property, add comment for missing BAR4]
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> Tested-by: Guenter Roeck <linux@roeck-us.net>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> Message-Id:
>> <17a50e58e6baa26440c7dac83f07fdbba1595439.1609191252.git.balaton@eik.bme.hu>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> 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 be09912b334..2d935b910f8 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));
>
> The if() here needs to be removed because the VIA IDE device starts up in
> legacy mode. If you look at the PCI bus master specification [1] the table at
> the top of page 3 is the breakdown of the PCI_CLASS_PROG byte: the configured
> value of 0x8a clearly indicates both channels are in legacy mode, which means
> that the PCI BARs are disabled and the controller is fixed to use the legacy
> IDE ioports with the option to switch up to native mode.
This if says that these ports are enabled when the legacy-mode flag is
set, i.e. we only emulate legacy-mode. It is disabled when we only emulate
half-native mode for pegasos2. If we leave it there unconditionally as you
suggest then we'll emulate (half-)native mode with the PCI BARs containing
these regs as well as having legacy ioports enabled at the same time. I
can't see how that's cleaner or more like what real hardware does.
> This is the main reason that the fuloong2e kernel panics on startup because
> its reads PCI_CLASS_PROG, discovers the controller is in legacy mode and then
> tries to access the legacy IDE ioports. Since the ioports are no longer
> mapped then the kernel crashes.
>
> (Fun fact: you can change PCI_CLASS_PROG to 0x8f and watch Linux configure
> the VIA IDE controller in native mode: it gets quite far before it decides to
> eventually panic)
>
> The fix here is simple: always enable the legacy IDE ioports. Zoltan
> mentioned he was concerned about having multiple IDE ports available, but
> this is not an issue for 2 reasons:
>
> 1) Before any of Zoltan's patches were applied, the VIA IDE driver set
> PCI_CLASS_PROG to 0x8a with the legacy IDE ports always enabled, and there
> were no issues
It did not run with pegasos2 back then, only with fuloong2e Linux and even
that wasn't tested very much, that's why my patches were needed in the
first place so of course there were no issues as it did not work at all
once something tried to use native mode like pegasos2 guests.
> 2) PCI bus enumeration always leaves the low end of IO space free so that
> there are no clashes with legacy ISA devices which can't be freely relocated
> in IO space
I don't know if that's a valid assumption to expect to be generally true
but I've tested pegasos2 with MorphOS after removing the if around
ide_init_ioport() above as you say and only leaving it around BAR4 and it
still boots, we end up with this memory map:
memory-region: pci1-io
0000000000000000-000000000000ffff (prio 0, i/o): pci1-io
0000000000000000-0000000000000007 (prio 0, i/o): dma-chan
0000000000000008-000000000000000f (prio 0, i/o): dma-cont
0000000000000020-0000000000000021 (prio 0, i/o): pic
0000000000000040-0000000000000043 (prio 0, i/o): pit
0000000000000060-0000000000000060 (prio 0, i/o): i8042-data
0000000000000064-0000000000000064 (prio 0, i/o): i8042-cmd
0000000000000070-0000000000000071 (prio 0, i/o): rtc
0000000000000070-0000000000000070 (prio 0, i/o): rtc-index
0000000000000081-0000000000000083 (prio 0, i/o): dma-page
0000000000000087-0000000000000087 (prio 0, i/o): dma-page
0000000000000089-000000000000008b (prio 0, i/o): dma-page
000000000000008f-000000000000008f (prio 0, i/o): dma-page
00000000000000a0-00000000000000a1 (prio 0, i/o): pic
00000000000000b2-00000000000000b3 (prio 0, i/o): apm-io
00000000000000c0-00000000000000cf (prio 0, i/o): dma-chan
00000000000000d0-00000000000000df (prio 0, i/o): dma-cont
* 0000000000000170-0000000000000177 (prio 0, i/o): ide
00000000000001ce-00000000000001d1 (prio 0, i/o): vbe
* 00000000000001f0-00000000000001f7 (prio 0, i/o): ide
00000000000002f8-00000000000002ff (prio 0, i/o): serial
* 0000000000000376-0000000000000376 (prio 0, i/o): ide
0000000000000378-000000000000037f (prio 0, i/o): parallel
00000000000003b4-00000000000003b5 (prio 0, i/o): vga
00000000000003ba-00000000000003ba (prio 0, i/o): vga
00000000000003c0-00000000000003cf (prio 0, i/o): vga
00000000000003d4-00000000000003d5 (prio 0, i/o): vga
00000000000003da-00000000000003da (prio 0, i/o): vga
00000000000003f1-00000000000003f5 (prio 0, i/o): fdc
* 00000000000003f6-00000000000003f6 (prio 0, i/o): ide
00000000000003f7-00000000000003f7 (prio 0, i/o): fdc
00000000000004d0-00000000000004d0 (prio 0, i/o): elcr
00000000000004d1-00000000000004d1 (prio 0, i/o): elcr
0000000000000f00-0000000000000fff (prio 0, i/o): via-pm
0000000000000f00-0000000000000f03 (prio 0, i/o): acpi-evt
0000000000000f04-0000000000000f05 (prio 0, i/o): acpi-cnt
0000000000000f08-0000000000000f0b (prio 0, i/o): acpi-tmr
0000000000001000-00000000000010ff (prio 1, i/o): alias ati.io @ati.mmregs 0000000000000000-00000000000000ff
0000000000002000-0000000000002007 (prio 1, i/o): via-ide0-data
0000000000003004-0000000000003007 (prio 1, i/o): via-ide0-cmd
0000000000005000-0000000000005007 (prio 1, i/o): via-ide1-data
0000000000006004-0000000000006007 (prio 1, i/o): via-ide1-cmd
0000000000008000-000000000000800f (prio 1, i/o): via-bmdma-container
0000000000008000-0000000000008003 (prio 0, i/o): via-bmdma
0000000000008004-0000000000008007 (prio 0, i/o): bmdma
0000000000008008-000000000000800b (prio 0, i/o): via-bmdma
000000000000800c-000000000000800f (prio 0, i/o): bmdma
0000000000009000-000000000000901f (prio 1, i/o): uhci
000000000000a000-000000000000a01f (prio 1, i/o): uhci
memory-region: system
0000000000000000-ffffffffffffffff (prio 0, i/o): system
0000000000000000-000000001fffffff (prio 0, ram): pegasos2.ram
0000000080000000-00000000bfffffff (prio 0, i/o): alias pci1-mem0-win @pci1-mem 0000000080000000-00000000bfffffff
00000000c0000000-00000000dfffffff (prio 0, i/o): alias pci0-mem0-win @pci0-mem 00000000c0000000-00000000dfffffff
00000000f1000000-00000000f100ffff (prio 0, i/o): mv64361
00000000f8000000-00000000f8ffffff (prio 0, i/o): alias pci0-io-win @pci0-io 0000000000000000-0000000000ffffff
00000000f9000000-00000000f9ffffff (prio 0, i/o): alias pci0-mem1-win @pci0-mem 0000000000000000-0000000000ffffff
00000000fd000000-00000000fdffffff (prio 0, i/o): alias pci1-mem1-win @pci1-mem 0000000000000000-0000000000ffffff
00000000fe000000-00000000feffffff (prio 0, i/o): alias pci1-io-win @pci1-io 0000000000000000-0000000000ffffff
00000000ff800000-00000000ffffffff (prio 0, i/o): alias pci1-mem3-win @pci1-mem 00000000ff800000-00000000ffffffff
00000000fff00000-00000000fff7ffff (prio 0, rom): pegasos2.rom
MorphOS uses the via-ide* BARs mapped at 2000-800f and hopefully does not
want to put anything or accidentally write to the legacy ide ports marked
with "*" above that I think should not be there (and with the if they are
not registered in this case) which I think is more like what real hardware
does because data sheet says in native mode it ignores legacy ports which
is not what your patch with removed if does.
>> 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(),
>> +};
>
> The other issue with the patch is that the legacy-mode property doesn't have
> anything to do with legacy mode: once the legacy IDE ioports are always
> enabled, the only thing this property controls is whether PCI BAR 4 is
> enabled. It doesn't select legacy mode since this is already the default, and
> neither does it force it by fixing the relevant bits in PCI_CLASS_PROG.
So then just rename it to break-native-mode or whatever you want, this is
not a problem with the patch itself, it's only broken after you remove the
if.
> PCI BAR 4 is for configuring BMDMA and Linux checks to see if BAR 4 is
> non-zero: if it is, then it tries to use BMDMA which is the feature that
> actually crashes the fuloong2e Linux kernel. Hence my suggestion to rename
That probably means it's not expecting to use anything but legacy mode on
fuloong2e but hard to tell without seeing it run on real machine.
> the property to "x-disable-bdma": the x- prefix convention clearly marks this
> out as an experimental property whilst also correctly describing what is is
> used for, just in case it leaks into external qdev configuration.
It's ok to change the property name, I think that can be done during
merging without having to test everything again as it can't break it but
for any other change please provide an alternative patch with your
suggestion, but also test it with both fuloong2e and pegasos2, The MorphOS
iso is freely downloadable and should work with -device
ati-vga,romfile=VGABIOS-lgpl-latest.bin as described on my qmiga.osdn.net
page. You can find a working version of pegasos2 emulation here:
https://osdn.net/projects/qmiga/scm/git/qemu/tree/pegasos2/
(which is currently still an older version but I plan to update it as I'm
splitting it up in smaller patches that I'll submit here later, but that
version should work for testing via-ide as nothing has changed in that
part). If a patch is posted here I may test it but I don't intend to
change this patch any more because I think what you propose here would not
make it any better and I don't know if Guenter wants to update it either.
>> 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 45c596f4fe5..d334fde389f 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");
>
> I hope this helps to summarise all my comments from the previous thread.
> Given that it is not possible to set default PCI BAR addresses in QEMU due to
> the post-qdev PCI bus reset, the ultimate aim should be to devise a couple of
> routines pci_enable_bar() and pci_disable_bar() so that the new property can
> be removed and controller mode set directly from configuration space writes
> to PCI_CLASS_PROG. And as per my reply in the previous thread, I can't see
> this happening in the near future.
As I wrote in previous reply I think it would be better to solve the
problem of removing or changing ISA devices instead which is more of a
problem than PCI BARs here but that's something that can be considered
later if we accept this as an interim solution now until a better
emulation can be written after the necessary changes are made elsewhere in
QEMU.
> I would really like to see a version of this patchset merged, particularly
> the integration test which will be of clear value moving forward. I am not
> against hacks like this where they are necessary, but as part of the review
> process they need to be understood with the aim being for QEMU to provide the
> guest with the most coherent environment possible.
I'm working on cleaning the pegasos2 patches so I can submit it here and
get it merged. I've already done the vt8231 part (of which the first
version of this via-ide patch I've posted in March was also part of but
only had time to do the rest now). This vt8231 model is the 24 patch
series I've posted yesterday (actually only 3 of those the rest is
cleaning up vt82c686b model so we can add vt8231 relatively cleanly) and
first half of that was alreay queued. Then there should be only two more
patches needed: one adding Marvell system controller chip and finally the
actual board code then we can make the acceptance test that would also
test via-ide with half-native mode as used on pegasos2.
I appreciate your review as long as it's about finding obviously wrong
details or suggesting improvements that can be implemented in at most a
few days (that's all what I can put in this now) without needing to change
anything unrelated to the actual patch. But saying that it is not
acceptable and all of this should be done in some completely different way
after rewriting half of device emulation in QEMU just to get rid of an if
or a flag is not a review that I consider helpful. It's OK to point out
how it could be improved later but make it clear that it should not hold
back progress and should not be something that prevents merging an
otherwise acceptable patch that does not have obviously wrong code or any
worse hacks than what we already have in QEMU now.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFC PATCH 3/5] hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified
2021-01-01 23:12 [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again Philippe Mathieu-Daudé
2021-01-01 23:12 ` [RFC PATCH 1/5] ide: Make room for flags in PCIIDEState and add one for legacy mode Philippe Mathieu-Daudé
2021-01-01 23:12 ` [RFC PATCH 2/5] via-ide: Fix fuloong2e support Philippe Mathieu-Daudé
@ 2021-01-01 23:12 ` Philippe Mathieu-Daudé
2021-01-01 23:19 ` Peter Maydell
2021-01-01 23:12 ` [RFC PATCH 4/5] tests/acceptance: Test boot_linux_console for fuloong2e Philippe Mathieu-Daudé
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-01 23:12 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Aleksandar Rikalo, qemu-block, Huacai Chen,
Mark Cave-Ayland, Philippe Mathieu-Daudé,
Artyom Tarasenko, Cleber Rosa, John Snow, Aurelien Jarno,
Wainer dos Santos Moschetta
Per the datasheet (Chapter 5.7.1. "PCI address regions"),
the PCIMAP register:
Map the 64Mbyte regions marked "PCI_Lo" in the CPU's memory map,
each of which can be assigned to any 64 Mbyte-aligned region of
PCI memory. The address appearing on the PCI bus consists of the
low 26 bits of the CPU physical address, with the high 6 bits
coming from the appropriate base6 field. Each of the three regions
is an independent window onto PCI memory, and can be positioned on
any 64Mbyte boundary in PCI space.
Remap the 3 regions on reset and when PCIMAP is updated.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/pci-host/bonito.c | 49 ++++++++++++++++++++++++++++++++------------
1 file changed, 36 insertions(+), 13 deletions(-)
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index a99eced0657..c58eeaf504c 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -137,6 +137,10 @@ FIELD(BONGENCFG, PCIQUEUE, 12, 1)
/* 4. PCI address map control */
#define BONITO_PCIMAP (0x10 >> 2) /* 0x110 */
+FIELD(PCIMAP, LO0, 0, 6)
+FIELD(PCIMAP, LO1, 6, 6)
+FIELD(PCIMAP, LO2, 12, 6)
+FIELD(PCIMAP, 2G, 18, 1)
#define BONITO_PCIMEMBASECFG (0x14 >> 2) /* 0x114 */
#define BONITO_PCIMAP_CFG (0x18 >> 2) /* 0x118 */
@@ -237,6 +241,7 @@ struct BonitoState {
qemu_irq *pic;
PCIBonitoState *pci_dev;
MemoryRegion pci_mem;
+ MemoryRegion pcimem_lo_alias[3];
};
#define TYPE_BONITO_PCI_HOST_BRIDGE "Bonito-pcihost"
@@ -245,6 +250,31 @@ OBJECT_DECLARE_SIMPLE_TYPE(BonitoState, BONITO_PCI_HOST_BRIDGE)
#define TYPE_PCI_BONITO "Bonito"
OBJECT_DECLARE_SIMPLE_TYPE(PCIBonitoState, PCI_BONITO)
+static void bonito_remap(PCIBonitoState *s)
+{
+ static const char *const region_name[3] = {
+ "pci.lomem0", "pci.lomem1", "pci.lomem2"
+ };
+ BonitoState *bs = BONITO_PCI_HOST_BRIDGE(s->pcihost);
+
+ for (size_t i = 0; i < 3; i++) {
+ uint32_t offset = extract32(s->regs[BONITO_PCIMAP], 6 * i, 6) << 26;
+
+ if (memory_region_is_mapped(&bs->pcimem_lo_alias[i])) {
+ memory_region_del_subregion(get_system_memory(),
+ &bs->pcimem_lo_alias[i]);
+ object_unparent(OBJECT(&bs->pcimem_lo_alias[i]));
+ }
+
+ memory_region_init_alias(&bs->pcimem_lo_alias[i], OBJECT(s),
+ region_name[i], &bs->pci_mem,
+ offset, 64 * MiB);
+ memory_region_add_subregion(get_system_memory(),
+ BONITO_PCILO_BASE + i * 64 * MiB,
+ &bs->pcimem_lo_alias[i]);
+ }
+}
+
static void bonito_writel(void *opaque, hwaddr addr,
uint64_t val, unsigned size)
{
@@ -260,7 +290,6 @@ static void bonito_writel(void *opaque, hwaddr addr,
case BONITO_BONPONCFG:
case BONITO_IODEVCFG:
case BONITO_SDCFG:
- case BONITO_PCIMAP:
case BONITO_PCIMEMBASECFG:
case BONITO_PCIMAP_CFG:
case BONITO_GPIODATA:
@@ -282,6 +311,10 @@ static void bonito_writel(void *opaque, hwaddr addr,
case BONITO_MEMSIZE:
s->regs[saddr] = val;
break;
+ case BONITO_PCIMAP:
+ s->regs[saddr] = val;
+ bonito_remap(s);
+ break;
case BONITO_BONGENCFG:
if (!(s->regs[saddr] & 0x04) && (val & 0x04)) {
reset = 1; /* bit 2 jump from 0 to 1 cause reset */
@@ -610,6 +643,8 @@ static void bonito_reset(void *opaque)
s->regs[BONITO_DQCFG] = 0x8;
s->regs[BONITO_MEMSIZE] = 0x10000000;
s->regs[BONITO_PCIMAP] = 0x6140;
+
+ bonito_remap(s);
}
static const VMStateDescription vmstate_bonito = {
@@ -626,7 +661,6 @@ static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
{
PCIHostState *phb = PCI_HOST_BRIDGE(dev);
BonitoState *bs = BONITO_PCI_HOST_BRIDGE(dev);
- MemoryRegion *pcimem_lo_alias = g_new(MemoryRegion, 3);
memory_region_init(&bs->pci_mem, OBJECT(dev), "pci.mem", BONITO_PCIHI_SIZE);
phb->bus = pci_register_root_bus(dev, "pci",
@@ -634,17 +668,6 @@ static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
dev, &bs->pci_mem, get_system_io(),
0x28, 32, TYPE_PCI_BUS);
- for (size_t i = 0; i < 3; i++) {
- char *name = g_strdup_printf("pci.lomem%zu", i);
-
- memory_region_init_alias(&pcimem_lo_alias[i], NULL, name,
- &bs->pci_mem, i * 64 * MiB, 64 * MiB);
- memory_region_add_subregion(get_system_memory(),
- BONITO_PCILO_BASE + i * 64 * MiB,
- &pcimem_lo_alias[i]);
- g_free(name);
- }
-
create_unimplemented_device("pci.io", BONITO_PCIIO_BASE, 1 * MiB);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RFC PATCH 3/5] hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified
2021-01-01 23:12 ` [RFC PATCH 3/5] hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified Philippe Mathieu-Daudé
@ 2021-01-01 23:19 ` Peter Maydell
2021-01-02 10:44 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 18+ messages in thread
From: Peter Maydell @ 2021-01-01 23:19 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Aleksandar Rikalo, Qemu-block, Huacai Chen, Mark Cave-Ayland,
QEMU Developers, Artyom Tarasenko, Cleber Rosa, John Snow,
Aurelien Jarno, Wainer dos Santos Moschetta
On Fri, 1 Jan 2021 at 23:12, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Per the datasheet (Chapter 5.7.1. "PCI address regions"),
> the PCIMAP register:
>
> Map the 64Mbyte regions marked "PCI_Lo" in the CPU's memory map,
> each of which can be assigned to any 64 Mbyte-aligned region of
> PCI memory. The address appearing on the PCI bus consists of the
> low 26 bits of the CPU physical address, with the high 6 bits
> coming from the appropriate base6 field. Each of the three regions
> is an independent window onto PCI memory, and can be positioned on
> any 64Mbyte boundary in PCI space.
>
> Remap the 3 regions on reset and when PCIMAP is updated.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/pci-host/bonito.c | 49 ++++++++++++++++++++++++++++++++------------
> 1 file changed, 36 insertions(+), 13 deletions(-)
>
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index a99eced0657..c58eeaf504c 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -137,6 +137,10 @@ FIELD(BONGENCFG, PCIQUEUE, 12, 1)
>
> /* 4. PCI address map control */
> #define BONITO_PCIMAP (0x10 >> 2) /* 0x110 */
> +FIELD(PCIMAP, LO0, 0, 6)
> +FIELD(PCIMAP, LO1, 6, 6)
> +FIELD(PCIMAP, LO2, 12, 6)
> +FIELD(PCIMAP, 2G, 18, 1)
> #define BONITO_PCIMEMBASECFG (0x14 >> 2) /* 0x114 */
> #define BONITO_PCIMAP_CFG (0x18 >> 2) /* 0x118 */
>
> @@ -237,6 +241,7 @@ struct BonitoState {
> qemu_irq *pic;
> PCIBonitoState *pci_dev;
> MemoryRegion pci_mem;
> + MemoryRegion pcimem_lo_alias[3];
> };
>
> #define TYPE_BONITO_PCI_HOST_BRIDGE "Bonito-pcihost"
> @@ -245,6 +250,31 @@ OBJECT_DECLARE_SIMPLE_TYPE(BonitoState, BONITO_PCI_HOST_BRIDGE)
> #define TYPE_PCI_BONITO "Bonito"
> OBJECT_DECLARE_SIMPLE_TYPE(PCIBonitoState, PCI_BONITO)
>
> +static void bonito_remap(PCIBonitoState *s)
> +{
> + static const char *const region_name[3] = {
> + "pci.lomem0", "pci.lomem1", "pci.lomem2"
> + };
> + BonitoState *bs = BONITO_PCI_HOST_BRIDGE(s->pcihost);
> +
> + for (size_t i = 0; i < 3; i++) {
> + uint32_t offset = extract32(s->regs[BONITO_PCIMAP], 6 * i, 6) << 26;
> +
> + if (memory_region_is_mapped(&bs->pcimem_lo_alias[i])) {
> + memory_region_del_subregion(get_system_memory(),
> + &bs->pcimem_lo_alias[i]);
> + object_unparent(OBJECT(&bs->pcimem_lo_alias[i]));
> + }
> +
> + memory_region_init_alias(&bs->pcimem_lo_alias[i], OBJECT(s),
> + region_name[i], &bs->pci_mem,
> + offset, 64 * MiB);
> + memory_region_add_subregion(get_system_memory(),
> + BONITO_PCILO_BASE + i * 64 * MiB,
> + &bs->pcimem_lo_alias[i]);
> + }
Rather than delete-and-reinit-and-add, it's probably better to
just create the subregions once at device startup, and then use
memory_region_set_enabled() and memory_region_set_address()
to manipulate whether the subregion is visible and what address
in the system memory it is mapped at.
thanks
-- PMM
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC PATCH 3/5] hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified
2021-01-01 23:19 ` Peter Maydell
@ 2021-01-02 10:44 ` Philippe Mathieu-Daudé
2021-01-02 10:56 ` Philippe Mathieu-Daudé
2021-01-02 11:22 ` BALATON Zoltan via
0 siblings, 2 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-02 10:44 UTC (permalink / raw)
To: Peter Maydell
Cc: Aleksandar Rikalo, Qemu-block, Huacai Chen, Mark Cave-Ayland,
QEMU Developers, Wainer dos Santos Moschetta, Aurelien Jarno,
Cleber Rosa, John Snow, Artyom Tarasenko
On 1/2/21 12:19 AM, Peter Maydell wrote:
> On Fri, 1 Jan 2021 at 23:12, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> Per the datasheet (Chapter 5.7.1. "PCI address regions"),
>> the PCIMAP register:
>>
>> Map the 64Mbyte regions marked "PCI_Lo" in the CPU's memory map,
>> each of which can be assigned to any 64 Mbyte-aligned region of
>> PCI memory. The address appearing on the PCI bus consists of the
>> low 26 bits of the CPU physical address, with the high 6 bits
>> coming from the appropriate base6 field. Each of the three regions
>> is an independent window onto PCI memory, and can be positioned on
>> any 64Mbyte boundary in PCI space.
>>
>> Remap the 3 regions on reset and when PCIMAP is updated.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> hw/pci-host/bonito.c | 49 ++++++++++++++++++++++++++++++++------------
>> 1 file changed, 36 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
>> index a99eced0657..c58eeaf504c 100644
>> --- a/hw/pci-host/bonito.c
>> +++ b/hw/pci-host/bonito.c
>> @@ -137,6 +137,10 @@ FIELD(BONGENCFG, PCIQUEUE, 12, 1)
>>
>> /* 4. PCI address map control */
>> #define BONITO_PCIMAP (0x10 >> 2) /* 0x110 */
>> +FIELD(PCIMAP, LO0, 0, 6)
>> +FIELD(PCIMAP, LO1, 6, 6)
>> +FIELD(PCIMAP, LO2, 12, 6)
>> +FIELD(PCIMAP, 2G, 18, 1)
>> #define BONITO_PCIMEMBASECFG (0x14 >> 2) /* 0x114 */
>> #define BONITO_PCIMAP_CFG (0x18 >> 2) /* 0x118 */
>>
>> @@ -237,6 +241,7 @@ struct BonitoState {
>> qemu_irq *pic;
>> PCIBonitoState *pci_dev;
>> MemoryRegion pci_mem;
>> + MemoryRegion pcimem_lo_alias[3];
>> };
>>
>> #define TYPE_BONITO_PCI_HOST_BRIDGE "Bonito-pcihost"
>> @@ -245,6 +250,31 @@ OBJECT_DECLARE_SIMPLE_TYPE(BonitoState, BONITO_PCI_HOST_BRIDGE)
>> #define TYPE_PCI_BONITO "Bonito"
>> OBJECT_DECLARE_SIMPLE_TYPE(PCIBonitoState, PCI_BONITO)
>>
>> +static void bonito_remap(PCIBonitoState *s)
>> +{
>> + static const char *const region_name[3] = {
>> + "pci.lomem0", "pci.lomem1", "pci.lomem2"
>> + };
>> + BonitoState *bs = BONITO_PCI_HOST_BRIDGE(s->pcihost);
>> +
>> + for (size_t i = 0; i < 3; i++) {
>> + uint32_t offset = extract32(s->regs[BONITO_PCIMAP], 6 * i, 6) << 26;
>> +
>> + if (memory_region_is_mapped(&bs->pcimem_lo_alias[i])) {
>> + memory_region_del_subregion(get_system_memory(),
>> + &bs->pcimem_lo_alias[i]);
>> + object_unparent(OBJECT(&bs->pcimem_lo_alias[i]));
>> + }
>> +
>> + memory_region_init_alias(&bs->pcimem_lo_alias[i], OBJECT(s),
>> + region_name[i], &bs->pci_mem,
>> + offset, 64 * MiB);
>> + memory_region_add_subregion(get_system_memory(),
>> + BONITO_PCILO_BASE + i * 64 * MiB,
>> + &bs->pcimem_lo_alias[i]);
>> + }
>
> Rather than delete-and-reinit-and-add, it's probably better to
> just create the subregions once at device startup, and then use
> memory_region_set_enabled() and memory_region_set_address()
> to manipulate whether the subregion is visible and what address
> in the system memory it is mapped at.
Great! Thanks Peter :) TIL these functions.
From what I understand from memory_region_readd_subregion (called
from memory_region_set_address) using memory_region_set_enabled()
directly is enough.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC PATCH 3/5] hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified
2021-01-02 10:44 ` Philippe Mathieu-Daudé
@ 2021-01-02 10:56 ` Philippe Mathieu-Daudé
2021-01-02 11:22 ` BALATON Zoltan via
1 sibling, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-02 10:56 UTC (permalink / raw)
To: Peter Maydell
Cc: Aleksandar Rikalo, Qemu-block, Huacai Chen, Mark Cave-Ayland,
QEMU Developers, Wainer dos Santos Moschetta, Artyom Tarasenko,
Cleber Rosa, John Snow, Aurelien Jarno
On 1/2/21 11:44 AM, Philippe Mathieu-Daudé wrote:
> On 1/2/21 12:19 AM, Peter Maydell wrote:
>> On Fri, 1 Jan 2021 at 23:12, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>>
>>> Per the datasheet (Chapter 5.7.1. "PCI address regions"),
>>> the PCIMAP register:
>>>
>>> Map the 64Mbyte regions marked "PCI_Lo" in the CPU's memory map,
>>> each of which can be assigned to any 64 Mbyte-aligned region of
>>> PCI memory. The address appearing on the PCI bus consists of the
>>> low 26 bits of the CPU physical address, with the high 6 bits
>>> coming from the appropriate base6 field. Each of the three regions
>>> is an independent window onto PCI memory, and can be positioned on
>>> any 64Mbyte boundary in PCI space.
>>>
>>> Remap the 3 regions on reset and when PCIMAP is updated.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> hw/pci-host/bonito.c | 49 ++++++++++++++++++++++++++++++++------------
>>> 1 file changed, 36 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
>>> index a99eced0657..c58eeaf504c 100644
>>> --- a/hw/pci-host/bonito.c
>>> +++ b/hw/pci-host/bonito.c
>>> @@ -137,6 +137,10 @@ FIELD(BONGENCFG, PCIQUEUE, 12, 1)
>>>
>>> /* 4. PCI address map control */
>>> #define BONITO_PCIMAP (0x10 >> 2) /* 0x110 */
>>> +FIELD(PCIMAP, LO0, 0, 6)
>>> +FIELD(PCIMAP, LO1, 6, 6)
>>> +FIELD(PCIMAP, LO2, 12, 6)
>>> +FIELD(PCIMAP, 2G, 18, 1)
>>> #define BONITO_PCIMEMBASECFG (0x14 >> 2) /* 0x114 */
>>> #define BONITO_PCIMAP_CFG (0x18 >> 2) /* 0x118 */
>>>
>>> @@ -237,6 +241,7 @@ struct BonitoState {
>>> qemu_irq *pic;
>>> PCIBonitoState *pci_dev;
>>> MemoryRegion pci_mem;
>>> + MemoryRegion pcimem_lo_alias[3];
>>> };
>>>
>>> #define TYPE_BONITO_PCI_HOST_BRIDGE "Bonito-pcihost"
>>> @@ -245,6 +250,31 @@ OBJECT_DECLARE_SIMPLE_TYPE(BonitoState, BONITO_PCI_HOST_BRIDGE)
>>> #define TYPE_PCI_BONITO "Bonito"
>>> OBJECT_DECLARE_SIMPLE_TYPE(PCIBonitoState, PCI_BONITO)
>>>
>>> +static void bonito_remap(PCIBonitoState *s)
>>> +{
>>> + static const char *const region_name[3] = {
>>> + "pci.lomem0", "pci.lomem1", "pci.lomem2"
>>> + };
>>> + BonitoState *bs = BONITO_PCI_HOST_BRIDGE(s->pcihost);
>>> +
>>> + for (size_t i = 0; i < 3; i++) {
>>> + uint32_t offset = extract32(s->regs[BONITO_PCIMAP], 6 * i, 6) << 26;
>>> +
>>> + if (memory_region_is_mapped(&bs->pcimem_lo_alias[i])) {
>>> + memory_region_del_subregion(get_system_memory(),
>>> + &bs->pcimem_lo_alias[i]);
>>> + object_unparent(OBJECT(&bs->pcimem_lo_alias[i]));
>>> + }
>>> +
>>> + memory_region_init_alias(&bs->pcimem_lo_alias[i], OBJECT(s),
>>> + region_name[i], &bs->pci_mem,
>>> + offset, 64 * MiB);
>>> + memory_region_add_subregion(get_system_memory(),
>>> + BONITO_PCILO_BASE + i * 64 * MiB,
>>> + &bs->pcimem_lo_alias[i]);
>>> + }
>>
>> Rather than delete-and-reinit-and-add, it's probably better to
>> just create the subregions once at device startup, and then use
>> memory_region_set_enabled() and memory_region_set_address()
>> to manipulate whether the subregion is visible and what address
>> in the system memory it is mapped at.
>
> Great! Thanks Peter :) TIL these functions.
> From what I understand from memory_region_readd_subregion (called
> from memory_region_set_address) using memory_region_set_enabled()
> directly is enough.
Actually since this is a alias, we don't want to modify the base
address but the alias offset, so I need to use
memory_region_set_alias_offset().
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC PATCH 3/5] hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified
2021-01-02 10:44 ` Philippe Mathieu-Daudé
2021-01-02 10:56 ` Philippe Mathieu-Daudé
@ 2021-01-02 11:22 ` BALATON Zoltan via
2021-01-02 13:10 ` Peter Maydell
1 sibling, 1 reply; 18+ messages in thread
From: BALATON Zoltan via @ 2021-01-02 11:22 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Aleksandar Rikalo, Qemu-block, Huacai Chen,
Mark Cave-Ayland, QEMU Developers, Wainer dos Santos Moschetta,
Artyom Tarasenko, Cleber Rosa, John Snow, Aurelien Jarno
[-- Attachment #1: Type: text/plain, Size: 4365 bytes --]
On Sat, 2 Jan 2021, Philippe Mathieu-Daudé wrote:
> On 1/2/21 12:19 AM, Peter Maydell wrote:
>> On Fri, 1 Jan 2021 at 23:12, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>>
>>> Per the datasheet (Chapter 5.7.1. "PCI address regions"),
>>> the PCIMAP register:
>>>
>>> Map the 64Mbyte regions marked "PCI_Lo" in the CPU's memory map,
>>> each of which can be assigned to any 64 Mbyte-aligned region of
>>> PCI memory. The address appearing on the PCI bus consists of the
>>> low 26 bits of the CPU physical address, with the high 6 bits
>>> coming from the appropriate base6 field. Each of the three regions
>>> is an independent window onto PCI memory, and can be positioned on
>>> any 64Mbyte boundary in PCI space.
>>>
>>> Remap the 3 regions on reset and when PCIMAP is updated.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> hw/pci-host/bonito.c | 49 ++++++++++++++++++++++++++++++++------------
>>> 1 file changed, 36 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
>>> index a99eced0657..c58eeaf504c 100644
>>> --- a/hw/pci-host/bonito.c
>>> +++ b/hw/pci-host/bonito.c
>>> @@ -137,6 +137,10 @@ FIELD(BONGENCFG, PCIQUEUE, 12, 1)
>>>
>>> /* 4. PCI address map control */
>>> #define BONITO_PCIMAP (0x10 >> 2) /* 0x110 */
>>> +FIELD(PCIMAP, LO0, 0, 6)
>>> +FIELD(PCIMAP, LO1, 6, 6)
>>> +FIELD(PCIMAP, LO2, 12, 6)
>>> +FIELD(PCIMAP, 2G, 18, 1)
>>> #define BONITO_PCIMEMBASECFG (0x14 >> 2) /* 0x114 */
>>> #define BONITO_PCIMAP_CFG (0x18 >> 2) /* 0x118 */
>>>
>>> @@ -237,6 +241,7 @@ struct BonitoState {
>>> qemu_irq *pic;
>>> PCIBonitoState *pci_dev;
>>> MemoryRegion pci_mem;
>>> + MemoryRegion pcimem_lo_alias[3];
>>> };
>>>
>>> #define TYPE_BONITO_PCI_HOST_BRIDGE "Bonito-pcihost"
>>> @@ -245,6 +250,31 @@ OBJECT_DECLARE_SIMPLE_TYPE(BonitoState, BONITO_PCI_HOST_BRIDGE)
>>> #define TYPE_PCI_BONITO "Bonito"
>>> OBJECT_DECLARE_SIMPLE_TYPE(PCIBonitoState, PCI_BONITO)
>>>
>>> +static void bonito_remap(PCIBonitoState *s)
>>> +{
>>> + static const char *const region_name[3] = {
>>> + "pci.lomem0", "pci.lomem1", "pci.lomem2"
>>> + };
>>> + BonitoState *bs = BONITO_PCI_HOST_BRIDGE(s->pcihost);
>>> +
>>> + for (size_t i = 0; i < 3; i++) {
>>> + uint32_t offset = extract32(s->regs[BONITO_PCIMAP], 6 * i, 6) << 26;
>>> +
>>> + if (memory_region_is_mapped(&bs->pcimem_lo_alias[i])) {
>>> + memory_region_del_subregion(get_system_memory(),
>>> + &bs->pcimem_lo_alias[i]);
>>> + object_unparent(OBJECT(&bs->pcimem_lo_alias[i]));
>>> + }
>>> +
>>> + memory_region_init_alias(&bs->pcimem_lo_alias[i], OBJECT(s),
>>> + region_name[i], &bs->pci_mem,
>>> + offset, 64 * MiB);
>>> + memory_region_add_subregion(get_system_memory(),
>>> + BONITO_PCILO_BASE + i * 64 * MiB,
>>> + &bs->pcimem_lo_alias[i]);
>>> + }
>>
>> Rather than delete-and-reinit-and-add, it's probably better to
>> just create the subregions once at device startup, and then use
>> memory_region_set_enabled() and memory_region_set_address()
>> to manipulate whether the subregion is visible and what address
>> in the system memory it is mapped at.
>
> Great! Thanks Peter :) TIL these functions.
> From what I understand from memory_region_readd_subregion (called
> from memory_region_set_address) using memory_region_set_enabled()
> directly is enough.
I have similar code in the series I've just posted where I'm mapping
regions of serial devices. I did consider using set_enabled and
set_address but ended up with removing and adding regions because I'm not
sure what happens if guest tries to move one region over another like
having one region at a default location while guest tries to map the other
one there (the pegasos2 maps serial at 0x2f8 which is normally COM2 on a
PC). This should not happen in theory but when removing disabled regions
it cannot happen so that looks safer therefore I chose to do that. Not
sure if this could be a problem here just shared my thughts about this.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC PATCH 3/5] hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified
2021-01-02 11:22 ` BALATON Zoltan via
@ 2021-01-02 13:10 ` Peter Maydell
2021-01-02 14:12 ` BALATON Zoltan via
0 siblings, 1 reply; 18+ messages in thread
From: Peter Maydell @ 2021-01-02 13:10 UTC (permalink / raw)
To: BALATON Zoltan
Cc: Aleksandar Rikalo, Qemu-block, Huacai Chen, Mark Cave-Ayland,
Philippe Mathieu-Daudé,
Wainer dos Santos Moschetta, QEMU Developers, Artyom Tarasenko,
Cleber Rosa, John Snow, Aurelien Jarno
On Sat, 2 Jan 2021 at 11:22, BALATON Zoltan <balaton@eik.bme.hu> wrote:
> I have similar code in the series I've just posted where I'm mapping
> regions of serial devices. I did consider using set_enabled and
> set_address but ended up with removing and adding regions because I'm not
> sure what happens if guest tries to move one region over another like
> having one region at a default location while guest tries to map the other
> one there (the pegasos2 maps serial at 0x2f8 which is normally COM2 on a
> PC). This should not happen in theory but when removing disabled regions
> it cannot happen so that looks safer therefore I chose to do that. Not
> sure if this could be a problem here just shared my thughts about this.
I'm not sure what you have in mind -- could you explain further?
There should be no difference as far as the MemoryRegion handling
code is concerned between "this memory region is marked disabled" and
"the memory region was deleted and will be created from fresh and added
back later" -- an MR that's in the hierarchy but not enabled is
entirely ignored, as if it wasn't there at all, when creating the
flat-view.
That said, doing memory_region_del_subregion()/memory_region_add_subregion()
I think is also OK -- what's definitely not required is actually
deleting and recreating the MRs the way this code is doing.
thanks
-- PMM
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH 3/5] hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified
2021-01-02 13:10 ` Peter Maydell
@ 2021-01-02 14:12 ` BALATON Zoltan via
0 siblings, 0 replies; 18+ messages in thread
From: BALATON Zoltan via @ 2021-01-02 14:12 UTC (permalink / raw)
To: Peter Maydell
Cc: Aleksandar Rikalo, Qemu-block, Huacai Chen, Mark Cave-Ayland,
Philippe Mathieu-Daudé,
Wainer dos Santos Moschetta, QEMU Developers, Artyom Tarasenko,
Cleber Rosa, John Snow, Aurelien Jarno
On Sat, 2 Jan 2021, Peter Maydell wrote:
> On Sat, 2 Jan 2021 at 11:22, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>> I have similar code in the series I've just posted where I'm mapping
>> regions of serial devices. I did consider using set_enabled and
>> set_address but ended up with removing and adding regions because I'm not
>> sure what happens if guest tries to move one region over another like
>> having one region at a default location while guest tries to map the other
>> one there (the pegasos2 maps serial at 0x2f8 which is normally COM2 on a
>> PC). This should not happen in theory but when removing disabled regions
>> it cannot happen so that looks safer therefore I chose to do that. Not
>> sure if this could be a problem here just shared my thughts about this.
>
> I'm not sure what you have in mind -- could you explain further?
> There should be no difference as far as the MemoryRegion handling
> code is concerned between "this memory region is marked disabled" and
> "the memory region was deleted and will be created from fresh and added
> back later" -- an MR that's in the hierarchy but not enabled is
> entirely ignored, as if it wasn't there at all, when creating the
> flat-view.
The device I was implementing has two registers one to set base address of
io region and another with bits to enable/disable the regions so I could
do set_address for base regs and set_enabled for control reg bits but I've
seen guests first flipping the enable bits on then setting the base
address so I thought it might cause problems with regions added to their
parent but thinking about it more it's probably the same if we remove
regions and add them instead of just set_enabled because they should be
readded when control reg bits are set so they'll end up at the same
default address.
> That said, doing memory_region_del_subregion()/memory_region_add_subregion()
> I think is also OK -- what's definitely not required is actually
> deleting and recreating the MRs the way this code is doing.
Anyway that's what I ended up doing and did not notice that this patch was
also deleting and recreating the memory regions which I did not do just
removing from parent when they are disabled but using set_address if they
are enabled and new base is set. Removing inactive regions maybe better
for debugging because they show up in info mtree so one can see which one
is enabled/disabled not sure how disabled regions show up though.
All in all I probably have nothing to add to this so just disregard my
comment.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFC PATCH 4/5] tests/acceptance: Test boot_linux_console for fuloong2e
2021-01-01 23:12 [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2021-01-01 23:12 ` [RFC PATCH 3/5] hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified Philippe Mathieu-Daudé
@ 2021-01-01 23:12 ` Philippe Mathieu-Daudé
2021-01-01 23:12 ` [RFC PATCH 5/5] tests/integration: Test Fuloong2E IDE drive, run userspace commands Philippe Mathieu-Daudé
2021-01-01 23:56 ` [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again BALATON Zoltan via
5 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-01 23:12 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Aleksandar Rikalo, qemu-block, Huacai Chen,
Mark Cave-Ayland, Philippe Mathieu-Daudé,
Willian Rampazzo, Artyom Tarasenko, Cleber Rosa, John Snow,
Aurelien Jarno, Wainer dos Santos Moschetta
From: Jiaxun Yang <jiaxun.yang@flygoat.com>
The kernel comes from debian archive so it's trusted.
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
Reviewed-by: Huacai Chen <chenhuacai@kernel.org>
Message-Id: <20201224031750.52146-9-jiaxun.yang@flygoat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/acceptance/boot_linux_console.py | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index cc6ec0f8c15..fb41bb7144b 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -170,6 +170,27 @@ def test_mips64el_malta(self):
console_pattern = 'Kernel command line: %s' % kernel_command_line
self.wait_for_console_pattern(console_pattern)
+ def test_mips64el_fuloong2e(self):
+ """
+ :avocado: tags=arch:mips64el
+ :avocado: tags=machine:fuloong2e
+ :avocado: tags=endian:little
+ """
+ deb_url = ('http://archive.debian.org/debian/pool/main/l/linux/'
+ 'linux-image-3.16.0-6-loongson-2e_3.16.56-1+deb8u1_mipsel.deb')
+ deb_hash = 'd04d446045deecf7b755ef576551de0c4184dd44'
+ deb_path = self.fetch_asset(deb_url, asset_hash=deb_hash)
+ kernel_path = self.extract_from_deb(deb_path,
+ '/boot/vmlinux-3.16.0-6-loongson-2e')
+
+ self.vm.set_console()
+ kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
+ self.vm.add_args('-kernel', kernel_path,
+ '-append', kernel_command_line)
+ self.vm.launch()
+ console_pattern = 'Kernel command line: %s' % kernel_command_line
+ self.wait_for_console_pattern(console_pattern)
+
def test_mips_malta_cpio(self):
"""
:avocado: tags=arch:mips
--
2.26.2
^ permalink raw reply related [flat|nested] 18+ messages in thread* [RFC PATCH 5/5] tests/integration: Test Fuloong2E IDE drive, run userspace commands
2021-01-01 23:12 [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2021-01-01 23:12 ` [RFC PATCH 4/5] tests/acceptance: Test boot_linux_console for fuloong2e Philippe Mathieu-Daudé
@ 2021-01-01 23:12 ` Philippe Mathieu-Daudé
2021-01-06 12:49 ` Willian Rampazzo
2021-01-01 23:56 ` [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again BALATON Zoltan via
5 siblings, 1 reply; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-01 23:12 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Aleksandar Rikalo, qemu-block, Guenter Roeck,
Huacai Chen, Mark Cave-Ayland, Philippe Mathieu-Daudé,
Artyom Tarasenko, Cleber Rosa, John Snow, Aurelien Jarno,
Wainer dos Santos Moschetta
Boot rootfs build by Guenter Roeck [*].
Full test output:
$ avocado --show=app,console run -t machine:fuloong2e tests/acceptance/
Fetching asset from tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_mips64el_fuloong2e
Fetching asset from tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_mips64el_fuloong2e
JOB ID : 4a89d17bd6fa596220113ea9b974412c37c6cf9a
JOB LOG : /home/phil/avocado/job-results/job-2021-01-01T23.54-4a89d17/job.log
(1/1) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_mips64el_fuloong2e:
console: [ 0.000000] Initializing cgroup subsys cpuset
console: [ 0.000000] Initializing cgroup subsys cpu
console: [ 0.000000] Initializing cgroup subsys cpuacct
console: [ 0.000000] Linux version 3.16.0-6-loongson-2e (debian-kernel@lists.debian.org) (gcc version 4.8.4 (Debian 4.8.4-1) ) #1 Debian 3.16.56-1+deb8u1 (2018-05-08)
console: [ 0.000000] memsize=256, highmemsize=0
console: [ 0.000000] CpuClock = 100000000
console: [ 0.000000] bootconsole [early0] enabled
console: [ 0.000000] CPU0 revision is: 00006302 (ICT Loongson-2)
console: [ 0.000000] FPU revision is: 00000501
console: [ 0.000000] Checking for the multiply/shift bug... no.
console: [ 0.000000] Checking for the daddiu bug... no.
console: [ 0.000000] Determined physical RAM map:
console: [ 0.000000] memory: 0000000010000000 @ 0000000000000000 (usable)
console: [ 0.000000] memory: 0000000004000000 @ 0000000010000000 (reserved)
console: [ 0.000000] memory: 0000000003ffffff @ 000000001c000001 (reserved)
console: [ 0.000000] Initrd not found or empty - disabling initrd
console: [ 0.000000] Zone ranges:
console: [ 0.000000] DMA [mem 0x00000000-0x00ffffff]
console: [ 0.000000] Normal [mem 0x01000000-0x0fffffff]
console: [ 0.000000] Movable zone start for each node
console: [ 0.000000] Early memory node ranges
console: [ 0.000000] node 0: [mem 0x00000000-0x0fffffff]
console: [ 0.000000] Reserving 0MB of memory at 0MB for crashkernel
console: [ 0.000000] Primary instruction cache 64kB, VIPT, direct mapped, linesize 32 bytes.
console: [ 0.000000] Primary data cache 64kB, 4-way, VIPT, no aliases, linesize 32 bytes
console: [ 0.000000] Unified secondary cache 512kB 4-way, linesize 32 bytes.
console: [ 0.000000] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 16327
console: [ 0.000000] Kernel command line: printk.time=0 console=ttyS0 root=/dev/sda ro panic=-1 noreboot
console: PID hash table entries: 1024 (order: -1, 8192 bytes)
console: Dentry cache hash table entries: 32768 (order: 4, 262144 bytes)
console: Inode-cache hash table entries: 16384 (order: 3, 131072 bytes)
console: Memory: 250752K/262144K available (5597K kernel code, 716K rwdata, 1548K rodata, 480K init, 581K bss, 11392K reserved)
console: NR_IRQS:128
console: Console: colour dummy device 80x25
console: Calibrating delay loop... 118.60 BogoMIPS (lpj=237216)
console: pid_max: default: 32768 minimum: 301
console: Security Framework initialized
console: AppArmor: AppArmor disabled by boot time parameter
console: Yama: disabled by default; enable with sysctl kernel.yama.*
console: Mount-cache hash table entries: 2048 (order: 0, 16384 bytes)
console: Mountpoint-cache hash table entries: 2048 (order: 0, 16384 bytes)
console: Initializing cgroup subsys memory
console: Initializing cgroup subsys devices
console: Initializing cgroup subsys freezer
console: Initializing cgroup subsys net_cls
console: Initializing cgroup subsys blkio
console: Initializing cgroup subsys perf_event
console: Initializing cgroup subsys net_prio
console: Checking for the daddi bug... no.
console: ftrace: allocating 17617 entries in 18 pages
console: devtmpfs: initialized
console: futex hash table entries: 256 (order: -2, 6144 bytes)
console: NET: Registered protocol family 16
console: vgaarb: loaded
console: SCSI subsystem initialized
console: PCI host bridge to bus 0000:00
console: pci_bus 0000:00: root bus resource [mem 0x14000000-0x1c000000]
console: pci_bus 0000:00: root bus resource [io 0x4000-0xffff]
console: pci_bus 0000:00: No busn resource found for root bus, will use [bus 00-ff]
console: via686b fix: ISA bridge
console: via686b fix: ISA bridge done
console: pci 0000:00:05.1: legacy IDE quirk: reg 0x10: [io 0x01f0-0x01f7]
console: pci 0000:00:05.1: legacy IDE quirk: reg 0x14: [io 0x03f6]
console: pci 0000:00:05.1: legacy IDE quirk: reg 0x18: [io 0x0170-0x0177]
console: pci 0000:00:05.1: legacy IDE quirk: reg 0x1c: [io 0x0376]
console: via686b fix: IDE
console: via686b fix: IDE done
console: pci 0000:00:05.4: quirk: [io 0xeee0-0xeeef] claimed by vt82c686 SMB
console: pci 0000:00:07.0: BAR 6: assigned [mem 0x14000000-0x1403ffff pref]
console: pci 0000:00:07.0: BAR 0: assigned [io 0x4000-0x40ff]
console: pci 0000:00:07.0: BAR 1: assigned [mem 0x14040000-0x140400ff]
console: pci 0000:00:05.2: BAR 4: assigned [io 0x4400-0x441f]
console: pci 0000:00:05.3: BAR 4: assigned [io 0x4420-0x443f]
console: Switched to clocksource MIPS
console: NET: Registered protocol family 2
console: TCP established hash table entries: 2048 (order: 0, 16384 bytes)
console: TCP bind hash table entries: 2048 (order: 0, 16384 bytes)
console: TCP: Hash tables configured (established 2048 bind 2048)
console: TCP: reno registered
console: UDP hash table entries: 512 (order: 0, 16384 bytes)
console: UDP-Lite hash table entries: 512 (order: 0, 16384 bytes)
console: NET: Registered protocol family 1
console: PCI: Enabling device 0000:00:05.2 (0000 -> 0001)
console: PCI: Enabling device 0000:00:05.3 (0000 -> 0001)
console: audit: initializing netlink subsys (disabled)
console: audit: type=2000 audit(1609541667.424:1): initialized
console: HugeTLB registered 32 MB page size, pre-allocated 0 pages
console: zbud: loaded
console: VFS: Disk quotas dquot_6.5.2
console: Dquot-cache hash table entries: 2048 (order 0, 16384 bytes)
console: msgmni has been set to 489
console: alg: No test for stdrng (krng)
console: Block layer SCSI generic (bsg) driver version 0.4 loaded (major 252)
console: io scheduler noop registered
console: io scheduler deadline registered
console: io scheduler cfq registered (default)
console: Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
console: console [ttyS0] disabled
console: serial8250.0: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A
console: console [ttyS0] enabled
console: console [ttyS0] enabled
console: bootconsole [early0] disabled
console: bootconsole [early0] disabled
console: pata_via 0000:00:05.1: BMDMA: BAR4 is zero, falling back to PIO
console: scsi0 : pata_via
console: scsi1 : pata_via
console: ata1: PATA max PIO4 cmd 0x1f0 ctl 0x3f6 irq 14
console: ata2: PATA max PIO4 cmd 0x170 ctl 0x376 irq 15
console: serio: i8042 KBD port at 0x60,0x64 irq 1
console: serio: i8042 AUX port at 0x60,0x64 irq 12
console: mousedev: PS/2 mouse device common for all mice
console: rtc_cmos rtc_cmos: rtc core: registered rtc_cmos as rtc0
console: rtc_cmos rtc_cmos: alarms up to one day, 114 bytes nvram
console: ata1.00: ATA-7: QEMU HARDDISK, 2.5+, max UDMA/100
console: ata1.00: 15320 sectors, multi 16: LBA48
console: ledtrig-cpu: registered to indicate activity on CPUs
console: TCP: cubic registered
console: NET: Registered protocol family 10
console: input: AT Raw Set 2 keyboard as /devices/platform/i8042/serio0/input/input0
console: ata1.00: configured for PIO4
console: mip6: Mobile IPv6
console: NET: Registered protocol family 17
console: mpls_gso: MPLS GSO support
console: registered taskstats version 1
console: rtc_cmos rtc_cmos: setting system clock to 2021-01-01 22:54:20 UTC (1609541660)
console: scsi 0:0:0:0: Direct-Access ATA QEMU HARDDISK 2.5+ PQ: 0 ANSI: 5
console: input: ImExPS/2 Generic Explorer Mouse as /devices/platform/i8042/serio1/input/input2
console: sd 0:0:0:0: [sda] 15320 512-byte logical blocks: (7.84 MB/7.48 MiB)
console: sd 0:0:0:0: [sda] Write Protect is off
console: sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
console: sda: unknown partition table
console: sd 0:0:0:0: [sda] Attached SCSI disk
console: EXT4-fs (sda): mounting ext3 file system using the ext4 subsystem
console: EXT4-fs (sda): mounted filesystem with ordered data mode. Opts: (null)
console: VFS: Mounted root (ext3 filesystem) readonly on device 8:0.
console: Freeing unused kernel memory: 480K (ffffffff808b8000 - ffffffff80930000)
console: random: nonblocking pool is initialized
console: EXT4-fs (sda): re-mounted. Opts: errors=remount-ro,data=ordered
console: Boot successful.
console: cat /proc/cpuinfo
console: / # cat /proc/cpuinfo
console: system type : lemote-fuloong-2e-box
console: machine : Unknown
console: processor : 0
console: cpu model : ICT Loongson-2 V0.2 FPU V0.1
console: BogoMIPS : 118.60
console: wait instruction : nouname -a
console: microsecond timers : yes
console: tlb_entries : 64
console: extra interrupt vector : no
console: hardware watchpoint : yes, count: 0, address/irw mask: []
console: isa : mips1 mips2 mips3
console: ASEs implemented :
console: shadow register sets : 1
console: kscratch registers : 0
console: package : 0
console: core : 0
console: VCED exceptions : not available
console: VCEI exceptions : not available
console: / # uname -a
console: Linux (none) 3.16.0-6-loongson-2e #1 Debian 3.16.56-1+deb8u1 (2018-05-08) mips64 GNU/Linux
console: reboot
console: / # reboot
console: / # umount: devtmpfs busy - remounted read-only
console: EXT4-fs (sda): re-mounted. Opts: (null)
console: The system is going down NOW!
console: Sent SIGTERM to all processes
console: Sent SIGKILL to all processes
console: Requesting system reboot
console: sd 0:0:0:0: [sda] Synchronizing SCSI cache
console: reboot: Restarting system
PASS (8.44 s)
RESULTS : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME : 8.85 s
[*] https://github.com/groeck/linux-build-test/
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Cc: Guenter Roeck <linux@roeck-us.net>
---
tests/acceptance/boot_linux_console.py | 30 ++++++++++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index fb41bb7144b..91fdd3c4cb2 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -183,13 +183,39 @@ def test_mips64el_fuloong2e(self):
kernel_path = self.extract_from_deb(deb_path,
'/boot/vmlinux-3.16.0-6-loongson-2e')
+ rootfs_url = ('https://github.com/groeck/linux-build-test/'
+ 'raw/8584a59ed9e5eb5ee7ca91f6d74bbb06619205b8/'
+ 'rootfs/mipsel64/rootfs.mipsel.ext3.gz')
+ rootfs_hash = '4316abb84b3b8384e124ada7fc72ef8cd5577dac'
+ rootfs_path_gz = self.fetch_asset(rootfs_url, asset_hash=rootfs_hash)
+ rootfs_path = os.path.join(self.workdir, 'rootfs.mipsel.ext3')
+ archive.gzip_uncompress(rootfs_path_gz, rootfs_path)
+
self.vm.set_console()
- kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
+ kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE
+ + 'console=ttyS0 '
+ + 'root=/dev/sda ro '
+ + 'panic=-1 noreboot')
self.vm.add_args('-kernel', kernel_path,
- '-append', kernel_command_line)
+ '-drive', 'if=none,format=raw,id=disk0,file='
+ + rootfs_path,
+ '-device', 'ide-hd,bus=ide.0,drive=disk0',
+ '-append', kernel_command_line,
+ '-no-reboot')
self.vm.launch()
+
console_pattern = 'Kernel command line: %s' % kernel_command_line
self.wait_for_console_pattern(console_pattern)
+ self.wait_for_console_pattern('Boot successful.')
+
+ exec_command_and_wait_for_pattern(self, 'cat /proc/cpuinfo',
+ 'BogoMIPS')
+ exec_command_and_wait_for_pattern(self, 'uname -a',
+ 'Debian')
+ exec_command_and_wait_for_pattern(self, 'reboot',
+ 'reboot: Restarting system')
+ # Wait for VM to shut down gracefully
+ self.vm.wait()
def test_mips_malta_cpio(self):
"""
--
2.26.2
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RFC PATCH 5/5] tests/integration: Test Fuloong2E IDE drive, run userspace commands
2021-01-01 23:12 ` [RFC PATCH 5/5] tests/integration: Test Fuloong2E IDE drive, run userspace commands Philippe Mathieu-Daudé
@ 2021-01-06 12:49 ` Willian Rampazzo
0 siblings, 0 replies; 18+ messages in thread
From: Willian Rampazzo @ 2021-01-06 12:49 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Aleksandar Rikalo, qemu-block, Huacai Chen,
Mark Cave-Ayland, qemu-devel, Wainer dos Santos Moschetta,
Aurelien Jarno, Artyom Tarasenko, Cleber Rosa, John Snow,
Guenter Roeck
On Fri, Jan 1, 2021 at 8:16 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Boot rootfs build by Guenter Roeck [*].
>
> Full test output:
>
> $ avocado --show=app,console run -t machine:fuloong2e tests/acceptance/
> Fetching asset from tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_mips64el_fuloong2e
> Fetching asset from tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_mips64el_fuloong2e
> JOB ID : 4a89d17bd6fa596220113ea9b974412c37c6cf9a
> JOB LOG : /home/phil/avocado/job-results/job-2021-01-01T23.54-4a89d17/job.log
> (1/1) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_mips64el_fuloong2e:
> console: [ 0.000000] Initializing cgroup subsys cpuset
> console: [ 0.000000] Initializing cgroup subsys cpu
> console: [ 0.000000] Initializing cgroup subsys cpuacct
> console: [ 0.000000] Linux version 3.16.0-6-loongson-2e (debian-kernel@lists.debian.org) (gcc version 4.8.4 (Debian 4.8.4-1) ) #1 Debian 3.16.56-1+deb8u1 (2018-05-08)
> console: [ 0.000000] memsize=256, highmemsize=0
> console: [ 0.000000] CpuClock = 100000000
> console: [ 0.000000] bootconsole [early0] enabled
> console: [ 0.000000] CPU0 revision is: 00006302 (ICT Loongson-2)
> console: [ 0.000000] FPU revision is: 00000501
> console: [ 0.000000] Checking for the multiply/shift bug... no.
> console: [ 0.000000] Checking for the daddiu bug... no.
> console: [ 0.000000] Determined physical RAM map:
> console: [ 0.000000] memory: 0000000010000000 @ 0000000000000000 (usable)
> console: [ 0.000000] memory: 0000000004000000 @ 0000000010000000 (reserved)
> console: [ 0.000000] memory: 0000000003ffffff @ 000000001c000001 (reserved)
> console: [ 0.000000] Initrd not found or empty - disabling initrd
> console: [ 0.000000] Zone ranges:
> console: [ 0.000000] DMA [mem 0x00000000-0x00ffffff]
> console: [ 0.000000] Normal [mem 0x01000000-0x0fffffff]
> console: [ 0.000000] Movable zone start for each node
> console: [ 0.000000] Early memory node ranges
> console: [ 0.000000] node 0: [mem 0x00000000-0x0fffffff]
> console: [ 0.000000] Reserving 0MB of memory at 0MB for crashkernel
> console: [ 0.000000] Primary instruction cache 64kB, VIPT, direct mapped, linesize 32 bytes.
> console: [ 0.000000] Primary data cache 64kB, 4-way, VIPT, no aliases, linesize 32 bytes
> console: [ 0.000000] Unified secondary cache 512kB 4-way, linesize 32 bytes.
> console: [ 0.000000] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 16327
> console: [ 0.000000] Kernel command line: printk.time=0 console=ttyS0 root=/dev/sda ro panic=-1 noreboot
> console: PID hash table entries: 1024 (order: -1, 8192 bytes)
> console: Dentry cache hash table entries: 32768 (order: 4, 262144 bytes)
> console: Inode-cache hash table entries: 16384 (order: 3, 131072 bytes)
> console: Memory: 250752K/262144K available (5597K kernel code, 716K rwdata, 1548K rodata, 480K init, 581K bss, 11392K reserved)
> console: NR_IRQS:128
> console: Console: colour dummy device 80x25
> console: Calibrating delay loop... 118.60 BogoMIPS (lpj=237216)
> console: pid_max: default: 32768 minimum: 301
> console: Security Framework initialized
> console: AppArmor: AppArmor disabled by boot time parameter
> console: Yama: disabled by default; enable with sysctl kernel.yama.*
> console: Mount-cache hash table entries: 2048 (order: 0, 16384 bytes)
> console: Mountpoint-cache hash table entries: 2048 (order: 0, 16384 bytes)
> console: Initializing cgroup subsys memory
> console: Initializing cgroup subsys devices
> console: Initializing cgroup subsys freezer
> console: Initializing cgroup subsys net_cls
> console: Initializing cgroup subsys blkio
> console: Initializing cgroup subsys perf_event
> console: Initializing cgroup subsys net_prio
> console: Checking for the daddi bug... no.
> console: ftrace: allocating 17617 entries in 18 pages
> console: devtmpfs: initialized
> console: futex hash table entries: 256 (order: -2, 6144 bytes)
> console: NET: Registered protocol family 16
> console: vgaarb: loaded
> console: SCSI subsystem initialized
> console: PCI host bridge to bus 0000:00
> console: pci_bus 0000:00: root bus resource [mem 0x14000000-0x1c000000]
> console: pci_bus 0000:00: root bus resource [io 0x4000-0xffff]
> console: pci_bus 0000:00: No busn resource found for root bus, will use [bus 00-ff]
> console: via686b fix: ISA bridge
> console: via686b fix: ISA bridge done
> console: pci 0000:00:05.1: legacy IDE quirk: reg 0x10: [io 0x01f0-0x01f7]
> console: pci 0000:00:05.1: legacy IDE quirk: reg 0x14: [io 0x03f6]
> console: pci 0000:00:05.1: legacy IDE quirk: reg 0x18: [io 0x0170-0x0177]
> console: pci 0000:00:05.1: legacy IDE quirk: reg 0x1c: [io 0x0376]
> console: via686b fix: IDE
> console: via686b fix: IDE done
> console: pci 0000:00:05.4: quirk: [io 0xeee0-0xeeef] claimed by vt82c686 SMB
> console: pci 0000:00:07.0: BAR 6: assigned [mem 0x14000000-0x1403ffff pref]
> console: pci 0000:00:07.0: BAR 0: assigned [io 0x4000-0x40ff]
> console: pci 0000:00:07.0: BAR 1: assigned [mem 0x14040000-0x140400ff]
> console: pci 0000:00:05.2: BAR 4: assigned [io 0x4400-0x441f]
> console: pci 0000:00:05.3: BAR 4: assigned [io 0x4420-0x443f]
> console: Switched to clocksource MIPS
> console: NET: Registered protocol family 2
> console: TCP established hash table entries: 2048 (order: 0, 16384 bytes)
> console: TCP bind hash table entries: 2048 (order: 0, 16384 bytes)
> console: TCP: Hash tables configured (established 2048 bind 2048)
> console: TCP: reno registered
> console: UDP hash table entries: 512 (order: 0, 16384 bytes)
> console: UDP-Lite hash table entries: 512 (order: 0, 16384 bytes)
> console: NET: Registered protocol family 1
> console: PCI: Enabling device 0000:00:05.2 (0000 -> 0001)
> console: PCI: Enabling device 0000:00:05.3 (0000 -> 0001)
> console: audit: initializing netlink subsys (disabled)
> console: audit: type=2000 audit(1609541667.424:1): initialized
> console: HugeTLB registered 32 MB page size, pre-allocated 0 pages
> console: zbud: loaded
> console: VFS: Disk quotas dquot_6.5.2
> console: Dquot-cache hash table entries: 2048 (order 0, 16384 bytes)
> console: msgmni has been set to 489
> console: alg: No test for stdrng (krng)
> console: Block layer SCSI generic (bsg) driver version 0.4 loaded (major 252)
> console: io scheduler noop registered
> console: io scheduler deadline registered
> console: io scheduler cfq registered (default)
> console: Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
> console: console [ttyS0] disabled
> console: serial8250.0: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A
> console: console [ttyS0] enabled
> console: console [ttyS0] enabled
> console: bootconsole [early0] disabled
> console: bootconsole [early0] disabled
> console: pata_via 0000:00:05.1: BMDMA: BAR4 is zero, falling back to PIO
> console: scsi0 : pata_via
> console: scsi1 : pata_via
> console: ata1: PATA max PIO4 cmd 0x1f0 ctl 0x3f6 irq 14
> console: ata2: PATA max PIO4 cmd 0x170 ctl 0x376 irq 15
> console: serio: i8042 KBD port at 0x60,0x64 irq 1
> console: serio: i8042 AUX port at 0x60,0x64 irq 12
> console: mousedev: PS/2 mouse device common for all mice
> console: rtc_cmos rtc_cmos: rtc core: registered rtc_cmos as rtc0
> console: rtc_cmos rtc_cmos: alarms up to one day, 114 bytes nvram
> console: ata1.00: ATA-7: QEMU HARDDISK, 2.5+, max UDMA/100
> console: ata1.00: 15320 sectors, multi 16: LBA48
> console: ledtrig-cpu: registered to indicate activity on CPUs
> console: TCP: cubic registered
> console: NET: Registered protocol family 10
> console: input: AT Raw Set 2 keyboard as /devices/platform/i8042/serio0/input/input0
> console: ata1.00: configured for PIO4
> console: mip6: Mobile IPv6
> console: NET: Registered protocol family 17
> console: mpls_gso: MPLS GSO support
> console: registered taskstats version 1
> console: rtc_cmos rtc_cmos: setting system clock to 2021-01-01 22:54:20 UTC (1609541660)
> console: scsi 0:0:0:0: Direct-Access ATA QEMU HARDDISK 2.5+ PQ: 0 ANSI: 5
> console: input: ImExPS/2 Generic Explorer Mouse as /devices/platform/i8042/serio1/input/input2
> console: sd 0:0:0:0: [sda] 15320 512-byte logical blocks: (7.84 MB/7.48 MiB)
> console: sd 0:0:0:0: [sda] Write Protect is off
> console: sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
> console: sda: unknown partition table
> console: sd 0:0:0:0: [sda] Attached SCSI disk
> console: EXT4-fs (sda): mounting ext3 file system using the ext4 subsystem
> console: EXT4-fs (sda): mounted filesystem with ordered data mode. Opts: (null)
> console: VFS: Mounted root (ext3 filesystem) readonly on device 8:0.
> console: Freeing unused kernel memory: 480K (ffffffff808b8000 - ffffffff80930000)
> console: random: nonblocking pool is initialized
> console: EXT4-fs (sda): re-mounted. Opts: errors=remount-ro,data=ordered
> console: Boot successful.
> console: cat /proc/cpuinfo
> console: / # cat /proc/cpuinfo
> console: system type : lemote-fuloong-2e-box
> console: machine : Unknown
> console: processor : 0
> console: cpu model : ICT Loongson-2 V0.2 FPU V0.1
> console: BogoMIPS : 118.60
> console: wait instruction : nouname -a
> console: microsecond timers : yes
> console: tlb_entries : 64
> console: extra interrupt vector : no
> console: hardware watchpoint : yes, count: 0, address/irw mask: []
> console: isa : mips1 mips2 mips3
> console: ASEs implemented :
> console: shadow register sets : 1
> console: kscratch registers : 0
> console: package : 0
> console: core : 0
> console: VCED exceptions : not available
> console: VCEI exceptions : not available
> console: / # uname -a
> console: Linux (none) 3.16.0-6-loongson-2e #1 Debian 3.16.56-1+deb8u1 (2018-05-08) mips64 GNU/Linux
> console: reboot
> console: / # reboot
> console: / # umount: devtmpfs busy - remounted read-only
> console: EXT4-fs (sda): re-mounted. Opts: (null)
> console: The system is going down NOW!
> console: Sent SIGTERM to all processes
> console: Sent SIGKILL to all processes
> console: Requesting system reboot
> console: sd 0:0:0:0: [sda] Synchronizing SCSI cache
> console: reboot: Restarting system
> PASS (8.44 s)
> RESULTS : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
> JOB TIME : 8.85 s
>
> [*] https://github.com/groeck/linux-build-test/
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Cc: Guenter Roeck <linux@roeck-us.net>
> ---
> tests/acceptance/boot_linux_console.py | 30 ++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again
2021-01-01 23:12 [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2021-01-01 23:12 ` [RFC PATCH 5/5] tests/integration: Test Fuloong2E IDE drive, run userspace commands Philippe Mathieu-Daudé
@ 2021-01-01 23:56 ` BALATON Zoltan via
2021-01-03 14:27 ` Mark Cave-Ayland
5 siblings, 1 reply; 18+ messages in thread
From: BALATON Zoltan via @ 2021-01-01 23:56 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Aleksandar Rikalo, qemu-block, Huacai Chen,
Mark Cave-Ayland, qemu-devel, Wainer dos Santos Moschetta,
Aurelien Jarno, Cleber Rosa, John Snow, Artyom Tarasenko
[-- Attachment #1: Type: text/plain, Size: 4919 bytes --]
On Sat, 2 Jan 2021, Philippe Mathieu-Daudé wrote:
> We closed 2020 with few discussions about the Fuloong 2E board
> (see [1] and [2]).
>
> This series collect the minimum set of patch to have the machine
> booting Linux guest again, including integration tests.
>
> This is sent as RFC because Mark raised some issues in (see [3]
> and previous in this thread) and I don't understand PCI enough
> to intervene.
Thanks for collecting these. Let me summarise the discussion because the
meaning may have been lost in the seamingly heated debate but I think
Mark's main concern was that he does not like having a feature flag and
property setting the emulation to partially emulate the device: either
only emulating legacy mode or native mode that this patch does but he
would prefer to faithfully emulate the device preferably allowing
switching between modes. But that's not easily possible without rewritig
either the ISA emulation or PCI emulation in QEMU because current code
does not allow these to be switched once created. That's way more work and
risk of breaking other things using these fundamental parts that I would
want to take on. My goal was only to allow using this (otherwise quite
unused and deglected) device model in pegasos2 emulation which needs
native mode. But turns out fuloong2e Linux wants legacy mode so we need a
way to resolve this conflict and the solution was this flag and keeping
partial emulation depending on machine.
But Mark still considered that a horrible hack but after looking more
closely he also found the difficulty of implementing a more faithful
emulation so he would accept the flag at the end but still wanted
registers to be set more consistently matching what the data sheet and
whatever ideals would dictate. However I've spent a lot of time before
finding these values that work with all clients and found some of these
clients have assumptions instead of working in an ideal world following
what data sheets say and I don't want to make any changes to this now
before we also have pegasos2 upstreamed so any change can be more
throughly tested and I don't have to retest everything for every small
change just to find something broke,
This was the main reason for disagreement and I think Mark's standards for
this device was way higher than necessary in this situation and I may have
got upset to have this pushed back again when we've already went through
this last March where we also had a long discussion after which Mark
managed to get rid of the flag but that now came back in a different form.
(Previously it was switching between fully native and non-100% native
mode, now it selects legacy or non-100% native mode where legacy is needed
for fuloong2e linux and non-100% native mode is needed for pegasos2
guests.) This may not be how the real device work (Mark also has concerns
about what exactly is non-100% native mode) and it may be a horrible hack
but it's probably the best that can be done with current QEMU facilities
and in the time I had and since this is only used on fuloong2e and
pegasos2 for a few obscure guests I think it does not need any more
complex solution at the moment.
It seems this disagreement on what's good enough for a device model to get
in QEMU is the source of disagreement between us with Mark but we'll sort
that out off list once I finish preparing my pegasos2 patches that will
finally show where these changes go and oters can also test any proposed
changes.
Regards,
BALATON Zoltan
> Peter commented a similar PCI issue with the Sam460ex [4] so might
> be able to help us here.
>
> Anyhow, sharing this PoC on the list with the test, the avoid boring
> manual testing.
>
> Regards,
>
> Phil.
>
> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg769105.html
> [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg769557.html
> [3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg769593.html
> [4] https://www.mail-archive.com/qemu-devel@nongnu.org/msg769697.html
>
> BALATON Zoltan (1):
> ide: Make room for flags in PCIIDEState and add one for legacy mode
>
> Guenter Roeck (1):
> via-ide: Fix fuloong2e support
>
> Jiaxun Yang (1):
> tests/acceptance: Test boot_linux_console for fuloong2e
>
> Philippe Mathieu-Daudé (2):
> hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified
> tests/integration: Test Fuloong2E IDE drive, run userspace commands
>
> include/hw/ide/pci.h | 7 +++-
> hw/ide/cmd646.c | 6 ++--
> hw/ide/via.c | 19 ++++++++--
> hw/mips/fuloong2e.c | 4 ++-
> hw/pci-host/bonito.c | 49 +++++++++++++++++++-------
> hw/sparc64/sun4u.c | 2 +-
> tests/acceptance/boot_linux_console.py | 47 ++++++++++++++++++++++++
> 7 files changed, 113 insertions(+), 21 deletions(-)
>
> --
> 2.26.2
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again
2021-01-01 23:56 ` [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again BALATON Zoltan via
@ 2021-01-03 14:27 ` Mark Cave-Ayland
2021-01-03 16:04 ` BALATON Zoltan via
0 siblings, 1 reply; 18+ messages in thread
From: Mark Cave-Ayland @ 2021-01-03 14:27 UTC (permalink / raw)
To: BALATON Zoltan, Philippe Mathieu-Daudé
Cc: Peter Maydell, Aleksandar Rikalo, qemu-block, Huacai Chen,
qemu-devel, Wainer dos Santos Moschetta, Artyom Tarasenko,
Cleber Rosa, John Snow, Aurelien Jarno
On 01/01/2021 23:56, BALATON Zoltan via wrote:
> But Mark still considered that a horrible hack but after looking more closely he also
> found the difficulty of implementing a more faithful emulation so he would accept the
> flag at the end but still wanted registers to be set more consistently matching what
> the data sheet and whatever ideals would dictate. However I've spent a lot of time
> before finding these values that work with all clients and found some of these
> clients have assumptions instead of working in an ideal world following what data
> sheets say and I don't want to make any changes to this now before we also have
> pegasos2 upstreamed so any change can be more throughly tested and I don't have to
> retest everything for every small change just to find something broke,
I'll reply briefly to this: from the latest analysis the part that's missing from
QEMU is the ability to disable/enable PCI BARs. But this is only something that has
come to light during the past week from Guenter's bug reports, as it is now clear the
Gentoo image you were using for a test case (which you also provided to me) was not
sufficient to test the VIA IDE functionality.
I've already said that we can make use of a temporary hack for now, but the patch in
its current form is still not right. I'll send a follow-up again to this thread so it
is in one place for Phil's reference.
ATB,
Mark.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again
2021-01-03 14:27 ` Mark Cave-Ayland
@ 2021-01-03 16:04 ` BALATON Zoltan via
0 siblings, 0 replies; 18+ messages in thread
From: BALATON Zoltan via @ 2021-01-03 16:04 UTC (permalink / raw)
To: Mark Cave-Ayland
Cc: Peter Maydell, Aleksandar Rikalo, qemu-block, Huacai Chen,
Philippe Mathieu-Daudé,
Wainer dos Santos Moschetta, qemu-devel, Artyom Tarasenko,
Cleber Rosa, John Snow, Aurelien Jarno
On Sun, 3 Jan 2021, Mark Cave-Ayland wrote:
> On 01/01/2021 23:56, BALATON Zoltan via wrote:
>> But Mark still considered that a horrible hack but after looking more
>> closely he also found the difficulty of implementing a more faithful
>> emulation so he would accept the flag at the end but still wanted registers
>> to be set more consistently matching what the data sheet and whatever
>> ideals would dictate. However I've spent a lot of time before finding these
>> values that work with all clients and found some of these clients have
>> assumptions instead of working in an ideal world following what data sheets
>> say and I don't want to make any changes to this now before we also have
>> pegasos2 upstreamed so any change can be more throughly tested and I don't
>> have to retest everything for every small change just to find something
>> broke,
>
> I'll reply briefly to this:
I'm also trying to keep on technical terms, will write a separate letter
off-list about the rest.
> from the latest analysis the part that's missing
> from QEMU is the ability to disable/enable PCI BARs. But this is only
> something that has come to light during the past week from Guenter's bug
It did come to light (at least to me) in first iteration of these via-ide
patches in March and I did tell you about it because I though it may help
finding a problem with CMD646 that is used on a Sparc machine that to my
knowledge you're interested in. See this thread:
https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg00776.html
(Probably that's how you got involved with the via-ide stuff in the first
place, as otherwise I've only cc-d you for CMD646 related changes.)
> reports, as it is now clear the Gentoo image you were using for a test case
> (which you also provided to me) was not sufficient to test the VIA IDE
> functionality.
That's true. It wasn't easy to find an image for fuloong2e as this was
only ever popular in China and has been long discontinued it seems so the
manufacturer's site that everything linked to was down. Therefore I could
only find this gentoo image that said it should work on real hardware.
Later Jiaxun and Huacai came back with other Linux images that we did not
have in March and Guenter updated his QEMU version now to find this
problem so now we have new, better test cases which showed we can't keep
the approach of only emulating (half-)native mode but also need legacy
mode for fuloong2e because while the beta gentoo kernel worked with native
mode other Linux kernels seem to want legacy mode on fuloong2e. (Pegasos2
guests still want half-native mode so we need both and can't keep the
original version that only emulated legacy mode.)
> I've already said that we can make use of a temporary hack for now, but the
> patch in its current form is still not right. I'll send a follow-up again to
> this thread so it is in one place for Phil's reference.
Thanks, I appreciate if this solution of having the legacy-mode flag can
get in now, even if you think it's not perfect. But it would allow to get
pegasos2 working while getting fuloong2e back the way it was and not
making it any worse than it was already (in fact I think it does improve
it a little even if not going the whole way). This can always be improved
later but I'd appreciate if you could also test your proposed solutions
with pegasos2 which will be easier once that's upstream otherwise I'll
have to do all the testing again and get pushed back from being able to
finally upstream this board that I've stared working on 2 years ago.
That's why I get upset if you demand more clean ups than absolutely
necessary to reach the minimum acceptable level.
If you can't wait until pegasos2 lands maybe you could experiment with
CMD646 which I think may be somewhat similar and used by boards you
maintain so it may be easier to experiment with without getting in the way
of each other. That one I think only emulates native mode that may be
enough for guests but also has a legacy mode that may be needed by some
boot loaders or different boards so you could try to find a way to
implement it cleanly in CMD646 then similar approach could be used for
via-ide.
As for the way to solve legacy/native mode switching I think going from
the ISA side is probably better than from the PCI side, i.e. instead of
disabling PCI BARs that you mention above it would be better to fix the
ISA emulation to allow deregistering previously added devices and allow
changing their parameters. I think so for the following reasons:
- PCI BAR regions are already disabled until something programs their BARs
and all of these IDE controllers start in legacy mode and guests change it
to native mode and unlikely to change back to legacy so we don't really
need to disable BARs once they are set up but we need to be able to turn
off legacy ISA IDE emulation when switching to native mode.
- Other parts of the via south bridge have similar problems that are ISA
devices that can be disabled or their base address changed such as serial,
parallel ports and FDC (I've found these while cleaning up vt82c686 as
part of merging my vt8231 emulation last week). So having a way to
enable/disable ISA devices or set their parameters after they are set up
could help better emulating those as well. (It may not be a problem though
as most guests set it up once at start and either use default values or
known values so we could map these there and get the guests work from
where it's questionable if it's worth the effort to emulate this better
but if QEMU had a way to do it in a simple way it could be done.)
- ISA is used by less machines then PCI so changing it probably has less
risk of breaking everything so may be easier to do. Although it is used by
the default pc machine and some fundamental machines so this should be
done more carefully than what I have time for. That's why I did not
attempt to do it and settled with what you call hacks to avoid having to
change a lot of low level QEMU stuff that would take ages to test, review
and get upstream. As long as those hacks work and not much worse than what
we already have in QEMU then this should not be a problem and these can be
cleaned up when somebody takes the time to clean up the other low level
parts.
I did not think about it much but I think the problem may be because of
ISA emulation dating back to the beginning and is still not fully
qdev-ified so it's using globals and has an API that was only designed for
creating devices at start but not change anything later. Qdev-ifying ISA
may be the way to go but that's a road I don't want to walk down as I'm
not interested in that apart from getting my guests boot and work so I'd
leave that to whoever is bothered enough by it to change it. I still think
cleaning it up now is not that high priority and could be done later when
more high priority issues are fixed (which I think are better sound and
gfx support for example) so spending time with those higher prirority
issues would be better than polishing low level stuff that at the end
don't make a difference in how guests run.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 18+ messages in thread