Hi Jonathan,
> Hi Zhao Liu
>
> I like the scheme. Strikes a good balance between complexity of description
> and systems that actually exist. Sure there are systems with more cache
> levels etc but they are rare and support can be easily added later
> if people want to model them.
Thanks for your support!
[snip]
> > +static int smp_cache_string_to_topology(MachineState *ms,
>
> Not a good name for a function that does rather more than that.
What about "smp_cache_get_valid_topology()"?
>
> > + char *topo_str,
> > + CPUTopoLevel *topo,
> > + Error **errp)
> > +{
> > + *topo = string_to_cpu_topo(topo_str);
> > +
> > + if (*topo == CPU_TOPO_LEVEL_MAX || *topo == CPU_TOPO_LEVEL_INVALID) {
> > + error_setg(errp, "Invalid cache topology level: %s. The cache "
> > + "topology should match the CPU topology level", topo_str);
> > + return -1;
> > + }
> > +
> > + if (!machine_check_topo_support(ms, *topo)) {
> > + error_setg(errp, "Invalid cache topology level: %s. The topology "
> > + "level is not supported by this machine", topo_str);
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void machine_parse_smp_cache_config(MachineState *ms,
> > + const SMPConfiguration *config,
> > + Error **errp)
> > +{
> > + MachineClass *mc = MACHINE_GET_CLASS(ms);
> > +
> > + if (config->l1d_cache) {
> > + if (!mc->smp_props.l1_separated_cache_supported) {
> > + error_setg(errp, "L1 D-cache topology not "
> > + "supported by this machine");
> > + return;
> > + }
> > +
> > + if (smp_cache_string_to_topology(ms, config->l1d_cache,
> > + &ms->smp_cache.l1d, errp)) {
>
> Indent is to wrong opening bracket.
> Same for other cases.
Could you please educate me about the correct style here?
I'm unsure if it should be indented by 4 spaces.
Thanks,
Zhao
