Re: [Qemu-devel] [PATCH v6 00/16] tcg/ppc: Add vector opcodes

[Mark Cave-Ayland]

On 03/09/2019 18:37, Aleksandar Markovic wrote:

> On Tue, Sep 3, 2019 at 7:05 PM Mark Cave-Ayland <
> address@hidden> wrote:
> 
>> On 01/07/2019 19:34, Howard Spoelstra wrote:
>>
>>> On Mon, Jul 1, 2019 at 12:30 PM Richard Henderson <
>>> address@hidden> wrote:
>>>
>>>> On 6/30/19 7:58 PM, Mark Cave-Ayland wrote:
>>>>> I don't have space for a full set of images on the G4, however I've
>>>> tried boot tests
>>>>> on installer CDs for MacOS 9, OS X 10.2, Linux and HelenOS and it looks
>>>> good here.
>>>>>
>>>>> Tested-by: Mark Cave-Ayland <address@hidden> [PPC32]
>>>>
>>>> Thanks!
>>>>
>>>> Hi
>>>
>>> I just compiled the v6 set applied to current master on my G5, Ubuntu 16.
>>> command line:
>>> ./qemu-system-ppc -L pc-bios -boot c m 512 -M mac99,via=pmu \
>>> -netdev user,id=net1 -device sungem,netdev=net1 \
>>> -drive file=10.3.img,format=raw,media=disk \
>>>
>>> With no specific cpu set, Mac OS 9.2 hard disk image and 9.2 iso do not
>> get
>>> to the desktop, they just hang while still in the openbios window. They
>>> need -cpu G4 on the command line to get to the desktop.
>>>
>>> OSX 10.3 installed image boots to desktop.
>>> OSX 10.3 iso boots to installer
>>> OSX 10.4 installed image boots to desktop.
>>> OSX 10.4 iso boot to installer
>>> OSX 10.5 installed image boots to desktop.
>>> OSX 10.5 iso boots to installer
>>>
>>> So there seems to be a difference between hosts: If ran on a G4 host
>> there
>>> is no need to add -cpu G4 to run Mac OS 9.x, while there is when ran on a
>>> G5 host.
>>
>> Are there any outstanding issues with this patchset now, or is it ready to
>> be merged?
>> I'm really looking forward to seeing the improved performance when testing
>> QEMU on my
>> Mac Mini :)
>>
>>
> Howard pointed to some illogical quirks of command line:
> 
>> If ran on a G4 host there is no need to add -cpu G4 to run Mac OS 9.x,
>> while there is when ran on a G5 host.
> 
> I am not sure if Howard says that this is a consequence of this series
> though.

No, that has been an existing issue for a long time :)

> Overall, I think this is a very good series - however, I had a number of
> minor
> objections to multiple patches, that don't affect (or affect in a minimal
> way)
> provided functionality - those objections are not addressed, nor properly
> discussed - but I do think they should be addressed in order to get the
> series
> in a better shape before upstreaming.

I've had a quick look at some of your review comments, and certainly I can see 
how
the earlier revisions have benefited from your feedback. There has been a lot of
positive discussion, and Richard has taken the time to respond and update the
patchset over several weeks to its latest revision.

AFAICT the only remaining issue is that related to the ISA flags, but to me this
isn't something that should prevent the patchset being merged. I can certainly 
see
how the current flags implementation may not be considered technically correct, 
but
then from your comments I don't see that it would be something that would be
particularly difficult to change at a later date either.

The things that are important to me are i) is the patchset functionally correct 
and
ii) is it understandable and maintainable. I would say that the first point is
clearly true (both myself and Howard have spent a lot of time testing it), and 
given
that I had to delve into these patches to fix the R2 register issue on 32-bit 
PPC
then I can confirm that the contents of the patches were a reasonably accurate
representation of the changes described within. And that's from someone like me 
who
is mostly still a TCG beginner :)

>From a slightly more selfish position as the PPC Mac machine maintainer, these
patches make a significant difference to me in that they reduce the MacOS boot 
times
during everyday testing. Now for someone like myself who works on QEMU as a 
hobby
outside of family life and a full time job, those few minutes are really 
important to
me and soon add up really quickly during testing.

I would really like these patches to be merged soon, since the worst thing that 
can
happen is that the patchset ends up bit-rotting and then all the time and 
effort put
into writing, testing and reviewing the patches by Richard, Howard, David, 
myself and
indeed your review time will all end up going to waste.


ATB,

Mark.