On Mon, 10 Oct 2011 10:57:34 +0100
"Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote:
> On Sat, Oct 08, 2011 at 12:12:46AM +0530, Prerna Saxena wrote:
> > This part of code primarily compares host and guest CPUs of a given
> > architecture for feature compatibility. x86 makes this choice based on
> > CPUID comparison.
> > Presently the PowerPC code has stubs to just get a 'pseries' guest to
> > boot. It would be augmented going forward, to do a detailed feature
> > comparison between guest and host CPUs on powerpc.
> > This part of code is presently well-classified into different
> > architectures, and consequently does not need reorganizing.
> >
> > ---
> > src/Makefile.am | 3 +-
> > src/cpu/cpu.c | 2 +
> > src/cpu/cpu_powerpc.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++
> > src/cpu/cpu_powerpc.h | 32 +++++++++++++++++++
> > 4 files changed, 117 insertions(+), 1 deletions(-)
> > create mode 100644 src/cpu/cpu_powerpc.c
> > create mode 100644 src/cpu/cpu_powerpc.h
>
> The idea here looks fine.
Thanks.
>
> > diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
> > new file mode 100644
> > index 0000000..6ceedc3
> > --- /dev/null
> > +++ b/src/cpu/cpu_powerpc.c
> > +
> > +#define VIR_FROM_THIS VIR_FROM_CPU
> > +
> > +static const char *archs[] = { "ppc64" };
>
> How about 'ppc' too ?
>
> > +static union cpuData *
> > +PowerPCNodeData(void)
> > +{
> > + union cpuData *data;
> > +
> > + if (VIR_ALLOC(data) < 0) {
> > + virReportOOMError();
> > + return NULL;
> > + }
> > +
> > + return data;
> > +}
> > +
> > +
> > +static int
> > +PowerPCDecode(virCPUDefPtr cpu,
> > + const union cpuData *data,
> > + const char **models,
> > + unsigned int nmodels,
> > + const char *preferred)
>
> Need to annotate these with 'ATTRIBUTE_UNUSED' to avoid compiler
> warnings.
Will do.
>
> > +{
> > + return 0;
> > +}
> > +
> > +static int
>
> Should be 'void'
Thanks for pointing this out, will fix it.
>
> > +PowerPCDataFree(union cpuData *data)
> > +{
> > + if (data == NULL)
> > + return 0;
> > +
> > + VIR_FREE(data);
> > +}
> > +
> > +struct cpuArchDriver cpuDriverPowerPC = {
> > + .name = "ppc64",
> > + .arch = archs,
> > + .narch = ARRAY_CARDINALITY(archs),
> > + .compare = NULL,
> > + .decode = PowerPCDecode,
> > + .encode = NULL,
> > + .free = PowerPCDataFree,
> > + .nodeData = PowerPCNodeData,
> > + .guestData = NULL,
> > + .baseline = NULL,
> > + .update = NULL,
> > + .hasFeature = NULL,
> > +};
>
> Should we have another copy for 'ppc' arch too ?
>
At present, I've tested this with KVM for Power ISA Book3S machines,
which introduces a new machine type "pseries" based on the ppc64
architecture. Extending this to 'ppc' architecture should be a trivial
effort -- I'll add that as soon as I get to test it.
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list