Re: [PATCH 0/7] hw/mips/malta: Rework to allow more than 2GB of RAM on 64-bit

[Philippe Mathieu-Daudé]

On 6/30/20 3:36 PM, Thomas Huth wrote:
> On 30/06/2020 13.17, Aleksandar Markovic wrote:
>>
>>
>> уторак, 30. јун 2020., Philippe Mathieu-Daudé <f4bug@amsat.org
>> <mailto:f4bug@amsat.org>> је написао/ла:
>>
>>     On 6/30/20 1:01 PM, Aleksandar Markovic wrote:
>>      >
>>      >
>>      > уторак, 30. јун 2020., Philippe Mathieu-Daudé <f4bug@amsat.org
>>     <mailto:f4bug@amsat.org>
>>      > <mailto:f4bug@amsat.org <mailto:f4bug@amsat.org>>> је написао/ла:
>>      >
>>      >     On Tue, Jun 30, 2020 at 12:52 PM Philippe Mathieu-Daudé
>>      >     <f4bug@amsat.org <mailto:f4bug@amsat.org>
>>     <mailto:f4bug@amsat.org <mailto:f4bug@amsat.org>>> wrote:
>>      >     >
>>      >     > On 6/30/20 12:48 PM, Aleksandar Markovic wrote:
>>      >     > >
>>      >     > >
>>      >     > > уторак, 30. јун 2020., Philippe Mathieu-Daudé
>>     <f4bug@amsat.org <mailto:f4bug@amsat.org>
>>      >     <mailto:f4bug@amsat.org <mailto:f4bug@amsat.org>>
>>      >     > > <mailto:f4bug@amsat.org <mailto:f4bug@amsat.org>
>>     <mailto:f4bug@amsat.org <mailto:f4bug@amsat.org>>>> је написао/ла:
>>      >     > >
>>      >     > >     Hi,
>>      >     > >
>>      >     > >     Following Jiaxun Yang's patch and discussion:
>>      >     > > https://patchwork.kernel.org/patch/11416915/
>>     <https://patchwork.kernel.org/patch/11416915/>
>>      >     <https://patchwork.kernel.org/patch/11416915/
>>     <https://patchwork.kernel.org/patch/11416915/>>
>>      >     > >     <https://patchwork.kernel.org/patch/11416915/
>>     <https://patchwork.kernel.org/patch/11416915/>
>>      >     <https://patchwork.kernel.org/patch/11416915/
>>     <https://patchwork.kernel.org/patch/11416915/>>>
>>      >     > >
>>      >     > >     - Rename the current machine as 'malta-virt' (keeping
>>      >     'malta' aliased)
>>      >     > >       Suggestions for better names are welcome, maybe
>>      >     'malta-unreal' or
>>      >     > >       'malta-unleashed' instead?
>>      >     > >     - Add 'malta-phys' which respects hardware
>>     restrictions (on
>>      >     RAM so far)
>>      >     > >     - Unleash 'malta-virt' to allow more than 2GB on
>> 64-bit
>>      >     > >
>>      >     > >     Philippe Mathieu-Daudé (7):
>>      >     > >       hw/mips/malta: Trivial code movement
>>      >     > >       hw/mips/malta: Register the machine as a TypeInfo
>>      >     > >       hw/mips/malta: Rename 'malta' machine as
>> 'malta-virt'
>>      >     > >       hw/mips/malta: Introduce
>> MaltaMachineClass::max_ramsize
>>      >     > >       hw/mips/malta: Introduce the 'malta-phys' machine
>>      >     > >       hw/mips/malta: Verify malta-phys machine uses
>>     correct DIMM
>>      >     sizes
>>      >     > >       hw/mips/malta: Allow more than 2GB on 64-bit
>> malta-virt
>>      >     > >
>>      >     > >      hw/mips/malta.c | 121
>>      >     +++++++++++++++++++++++++++++++++++++++---------
>>      >     > >      1 file changed, 99 insertions(+), 22 deletions(-)
>>      >     > >
>>      >     > >     --
>>      >     > >
>>      >     > >
>>      >     > >
>>      >     > > Thank you, Philippe, for providing this series.
>>      >     > >
>>      >     > > However, in previous discussion on the patch you mention
>>     above, I
>>      >     > > already expressed serious reservations on the approach
>>     taken in that
>>      >     > > patch. These reservations stay today too.
>>      >     > >
>>      >     > > There is nothing qualitatively different between the
>> original
>>      >     patch and
>>      >     > > this series. Naming and related stuff are just cosmetic
>>     issues.
>>      >     >
>>      >     > OK, what about considering all patches except the last one?
>>      >     > So we can run firmware on a real Malta board, not the QEMU
>>      >     > imaginary one (in the discussion you said QEMU should
>> respect
>>      >     > real hardware, which I agree).
>>      >     >
>>      >     > >
>>      >     > > The good thing about this series is that one can apply it
>>      >     dowstream, if
>>      >     > > one finds it useful. However, it is not suitable for
>>     upstreaming
>>      >
>>      >     IOW, what is missing to have this series (except the last
>> patch)
>>      >     accepted upstream?
>>      >
>>      >
>>      > It is not what is missing, but was already is in the series that
>>     makes
>>      > it not suitable for upstreaming. The very concept of the series is
>>      > problematic.
>>
>>     What is the series is not suitable for upstream? Can you point to
>>     patch and code please? How would you conceptually resolve the
>>     problem?
>>
>>
>> The answer is already in the original thread on the original patch.
>>
>> The problem is artificialy imposed:
>>
>> One needs a feature not present in the physical system. The solution
>> is not in creating "virtual" upgrade - this violates the basic
>> foundation if QEMU.
>>
>> If the feature is missing, the optimal solution would be emulating the
>> physical solution that has that feature.
>>
>> In some rare cases (that should be avoided as mush as possible), and
>> for really good reasons, we can create an emulation of some imagined
>> hardware that was never designed let alone built - there are a couple
>> of examples in other targets.
>>
>> But extending the emulation of existing physical system with
>> non-existing features is not acceptable.
> 
> Interesting statement. I suggest to include the following patch in your
> next mips pull request to avoid that mips users spoil their machines
> with virtual-only features:
> 
> diff a/default-configs/mips-softmmu-common.mak
> b/default-configs/mips-softmmu-common.mak
> --- a/default-configs/mips-softmmu-common.mak
> +++ b/default-configs/mips-softmmu-common.mak
> @@ -6,10 +6,10 @@ CONFIG_SEMIHOSTING=y
>  CONFIG_ISA_BUS=y
>  CONFIG_PCI=y
>  CONFIG_PCI_DEVICES=y
> +CONFIG_VIRTIO_PCI=n
>  CONFIG_VGA_ISA=y
>  CONFIG_VGA_ISA_MM=y
>  CONFIG_VGA_CIRRUS=y
> -CONFIG_VMWARE_VGA=y
>  CONFIG_SERIAL=y
>  CONFIG_SERIAL_ISA=y
>  CONFIG_PARALLEL=y
> 
> ;-)
> 
> No, seriously, as far as I can tell, QEMU was never in that "we strictly
> only emulate real hardware" camp, even the homepage talks about
> "virtualizer".
> 
> But introducing a separate machine for this feature sounds a little bit
> excessive for me, too. What about introducing a machine property for the
> max-ram-size? With the default value, the machine could restrict the ram
> sizes to the values that are possible with the real hardware, and only
> if the (advanced) user tweaks the property, it would be possible to set
> bigger values. Just my 0.02 €.
> 
>  Thomas
> 
> 
> PS: Why does mips use CONFIG_VMWARE_VGA=y at all? Is VMWARE also
> available for mips?
> 
> PPS: Why is mips still not using proper Kconfig settings yet?

Related to "ACPI + ICH9 + PIIX4" interdependence with X86. Fixing MIPS
would break X86. We can not break X86 (and there is little interest from
enterprise X86 on solving MIPS emulation problems).

Then I consumed all the time my manager granted me to work on it, so
it is very low in my list. If someone else want to do it, help welcomed.