Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic

[Greg Kurz <groug@kaod.org>]

On Wed, 7 Sep 2016 15:10:59 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 07/09/2016 14:44, Greg Kurz wrote:
> > On Wed, 7 Sep 2016 11:36:21 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >   
> >> On 06/09/2016 23:41, Greg Kurz wrote:  
> >>> On Tue,  6 Sep 2016 15:17:56 +0200
> >>> Laurent Vivier <lvivier@redhat.com> wrote:
> >>>     
> >>>> And add support for ppc64.
> >>>>
> >>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >>>> ---
> >>>> v2:
> >>>> - remove useless parenthesis, inline
> >>>>    
> >>>
> >>> This works indeed but I'm just feeling curious about the QOSOps type introduced
> >>> by the following commit:
> >>>
> >>> commit 90e5add6f2fa0b0bd9a4c1d5a4de2304b5f3e466
> >>> Author: John Snow <jsnow@redhat.com>
> >>> Date:   Mon Jan 19 15:15:55 2015 -0500
> >>>
> >>>     libqos: add pc specific interface
> >>>
> >>> Wouldn't this be better to implement something similar for ppc64 instead of
> >>> relying on strcmp() ?    
> >>
> >> Tests can be generic and to be run on several archs: we need the
> >> strcmp() to check the guest arch [1], it can't be hardcoded in the test.
> >>  
> > 
> > I agree for truely platform agnostic tests, but this is obviously not the case
> > for RTAS, which is the goal of this series.
> > 
> > My suggestion is basically to:
> > - keep malloc-ppc64.[ch] from your series
> > - introduce libqos-ppc64.[ch] like the existing libqos-pc.[ch]
> > - add qtest_ppc64_[start|end]() wrappers to pass global_qtest to
> >   qtest_ppc64_[boot|shutdown]()
> > - adapt the final RTAS test patch to use these wrappers and q[malloc|free]()  
> 
> You're right it's the good way to implement guest memory allocation...
> I'm going to add this part in the series.
> 
> > BTW, maybe s/ppc64/ppc to match hw/ppc, since libqos is about HW platforms,
> > not target archs.  
> 
> I use ppc64 because we guess guest memory is at least 256MB, and this is
> true only with ppc64/pseries. With ppc, I think we should use qfw_cfg()
> as for PC.
> 

True. In this case, maybe you should even use spapr, since RTAS is for pseries
only, and so will be the PCI bits you mention in the cover letter.

Cheers.

--
Greg

> Thanks,
> Laurent
> > This is more work, but I guess in the end it maybe useful in the long term.
> > 
> > And, of course, I'm volunteering to participate, with patches/reviewing/testing.
> > 
> > Makes sense ?
> > 
> > Cheers.
> > 
> > --
> > Greg
> >   
> >> Thanks,
> >> Laurent
> >>
> >> [1]
> >> const char *qtest_get_arch(void)
> >> {
> >>     const char *qemu = getenv("QTEST_QEMU_BINARY");
> >>     g_assert(qemu != NULL);
> >>     const char *end = strrchr(qemu, '/');
> >>
> >>     return end + strlen("/qemu-system-");
> >> }  
> >