[RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again

Philippe Mathieu-Daudé posted 5 patches 4 years, 10 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210101231215.1870611-1-f4bug@amsat.org
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(-)
[RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again
Posted by Philippe Mathieu-Daudé 4 years, 10 months ago
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.

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

Re: [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again
Posted by BALATON Zoltan via 4 years, 10 months ago
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
>
>
>
Re: [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again
Posted by Mark Cave-Ayland 4 years, 10 months ago
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.

Re: [RFC PATCH 0/5] hw/mips: Fix Fuloong2E to boot Linux guest again
Posted by BALATON Zoltan via 4 years, 10 months ago
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