On Tue, Apr 02, 2019 at 11:46:37AM +0200, Igor Mammedov wrote: > On Tue, 2 Apr 2019 12:42:37 +0800 > Tao Xu <tao3...@intel.com> wrote: > > > On 2/6/19 6:11 PM, Igor Mammedov wrote: > > > On Thu, 31 Jan 2019 15:16:54 +0800 > > > Tao Xu <tao3...@intel.com> wrote: > > > > > >> From: Liu Jingqi <jingqi....@intel.com> > > >> > > >> Add -numa hmat-lb option to provide System Locality Latency and > > >> Bandwidth Information. These memory attributes help to build > > >> System Locality Latency and Bandwidth Information Structure(s) > > >> in ACPI Heterogeneous Memory Attribute Table (HMAT). > > > Maybe instead of adding/using more globals since the rest of numa.c was > > > written so, it's time to introduce NumaMachine type which inherits > > > form base Machine and extends it with numa extensions. > > > > > > You don't have to refactor all old numa code for that (just the parts > > > you use in HMAT) and later we could gradually refactor the rest of > > > numa handling. > > > > > Hi Igor, > > > > I am sorry for replying after so long, because I am preparing the next > > version of patch and confusing about the implementation of NumaMachine. > > If NumaMachine inherits from Machine, and then the PCmachine and other > > platform machine inherits from NumaMachine (Implementation a)? Or > > PCmachine and other platform machine inherits from Machine > > (Implementation b)? > > > > Implementation a: Implementation b: > > Machine Machine > > │ │ > > └── NumaMachine ├── NumaMachine > > │ │ > > ├── PCMachine ├── PCMachine > > │ │ > > └── none-PCMachine └── none-PCMachine > > I'd imagine 'a', where none-PCMachine is arm/virt and spapr machines that > support numa today. The rest would inherit from plain Machine. > > > And how about add numa.c's globals into MachineState? This may not > > change a lot. > That's another possible way, but it's polluting generic machine with > numa specifics about which only 3-4 boards care. > From my point of view it's a bit better then globals but not much.
I suggest following the pattern used for MachineState::device_memory and MachineState::nvdimms_state (pointer to optional feature-specific state) intead of inheritance. -- Eduardo