Re: [PATCH qemu v19] spapr: Implement Open Firmware client interface

[BALATON Zoltan]

On Mon, 24 May 2021, David Gibson wrote:
> On Mon, May 17, 2021 at 02:17:36PM +0200, BALATON Zoltan wrote:
> > On Mon, 17 May 2021, Alexey Kardashevskiy wrote:
> > > On 5/16/21 01:04, BALATON Zoltan wrote:
> > > > On Thu, 22 Apr 2021, Alexey Kardashevskiy wrote:
> > >
> > > [snip]
> > >
> > > > > +/* Defined as Big Endian */
> > > > > +struct prom_args {
> > > > > +    uint32_t service;
> > > > > +    uint32_t nargs;
> > > > > +    uint32_t nret;
> > > > > +    uint32_t args[10];
> > > > > +} QEMU_PACKED;
> > > >
> > > > This #define and struct definition should probably be moved to
> > > > include/hw/ppc/vof.h as I had to copy these when trying to get VOF
> > > > running with pegasos2 and these seem to be VOF specific not spapr.
> > >
> > > Correct, I'll fix it - there are 2 copies already.
> > >
> > >
> > > > I was trying to wire up VOF on pegasos2 as proof of concept but I
> > > > did not get very far as it crashed at the first move due to using
> > > > mtmsrd which does not exist on the 32 bit CPUs (G4 or G3) used by
> > > > pegasos2:
> > > >
> > > > vof_claim virt=0x0 size=0xc38 align=0x0 => 0x0
> > > > vof_claim virt=0x0 size=0x8000 align=0x8000 => 0x8000
> > > > vof_claim virt=0xc00000 size=0x18fd62 align=0x0 => 0xc00000
> > > > vof_claimed 0x0..0xc38 size=0xc38
> > > > vof_claimed 0x8000..0x10000 size=0x8000
> > > > vof_claimed 0xc00000..0xd8fd62 size=0x18fd62
> > > > vof_avail 0xc38..0x8000 size=0x73c8
> > > > vof_avail 0x10000..0xc00000 size=0xbf0000
> > > > vof_avail 0xd8fd62..0x20000000 size=0x1f27029e
> > > > via_superio_cfg: unimplemented register 0xf2
> > > > via_superio_cfg: unimplemented register 0xf4
> > > > via_superio_cfg: unimplemented register 0xf6
> > > > via_superio_cfg: unimplemented register 0xf7
> > > > invalid/unsupported opcode: 1f - 12 - 05 - 00 (7fe00164) fff00108 0
> > > > ----------------
> > > > IN:
> > > > 0xfff00100:  3fe00000  lis      r31, 0
> > > > 0xfff00104:  63ff0000  ori      r31, r31, 0
> > > > 0xfff00108:  7fe00164  mtmsrd   r31
> > > >
> > > > ----------------
> > > > IN:
> > > > 0xfff00700:  807e8020  lwz      r3, -0x7fe0(r30)
> > > > 0xfff00704:  4cc63182  crclr    6
> > > > 0xfff00708:  4bfffd1d  bl       0xfff00424
> > > >
> > > > Invalid access at addr 0xFFFF8020, size 4, region '(null)', reason:
> > > > rejected
> > > >
> > > > Is this mtmsrd really needed? Do 64-bit Power CPUs start in 64 bit mode?
> > >
> > > Yup:
> > > https://git.qemu.org/?p=qemu.git;a=blob;f=target/ppc/translate_init.c.inc;h=66e6a4a746f46148e0006081af09391b32c125cd;hb=HEAD#l10085
> > >
> > > I cannot find the exact reason for that, probably PAPR or some PPC-OF
> > > binding says so.
> > >
> > >
> > > > I'd expect them to be in compatibility mode unless 64 bit is enabled
> > > > but I did not check the docs. If it's needed maybe a dummy handler
> > > > has to be at 0x700 to ignore this exception if it's running on a
> > > > 32-bit CPU.
> > >
> > > I wanted MSR and the code to be in sync explicitly.
> >
> > OK, then can you add a dummy exception handler at 0x700 to ignore this so it
> > would also work on a 32-bit CPU? That does not seem to be too difficult.
>
> I don't like that idea.  I really think the idea with VOF should be
> that it's explicitly closely married to qemu, so we shouldn't have to
> mess around with the firmware image coping with different situations.
>
> So a) I don't think we should go to *any* trouble to have the same VOF
> guest image work on multiple machine types (nor multiple qemu
> versions).  And b) I think VOF should just assume qemu has started it
> in the mode it wants and not try to get it there.

This problem is already resolved by the latest vof patch (v20), the idea 
of an exception handler came back because of sc 1 causing program 
exception after vhyp is cleared but if we can keep that enabled this won't 
be needed. VOF itself is compiled 32 bit so should work on ppc32 just 
there were some 64-bit opcodes left that can be eliminated so we don't 
need another version for pegasos2. I think it's simpler to maintain a 
single vof version than having a fork for every machine that tries to use 
it so while not going into big trouble is OK but some small changes to 
avoid a separate version should be considered. So far I haven't found 
anything that would need big changes. Even if we decide to use a different 
hypercall method in the future that could be implemented in the same vof 
sources either compile or run time selectable but for now I hope the 
current way will work, I'll have to test it further.

Regards,
BALATON Zoltan