On Thu, Jul 31, 2025 at 12:11:41AM +0800, Xiaoyao Li wrote: > Date: Thu, 31 Jul 2025 00:11:41 +0800 > From: Xiaoyao Li <xiaoyao...@intel.com> > Subject: Re: [PATCH 1/2] i386/cpu: Enable SMM cpu addressspace > > On 7/30/2025 11:20 PM, Zhao Liu wrote: > > > > > + cpu_address_space_init(cpu, 1, "cpu-smm", &smram_as_root); > > > > > > > > It is worth mentioning in the commit message that directly sharing > > > > MemoryRegion in CPUAddressSpace is safe. > > > > > > It's unnecessary to me. It's common that different Address space share the > > > same (root) memory region. e.g., for address space 0 for the cpu, though > > > what passed in is cpu->memory, they all point to system_memory. > > > > For cpu->memory, there's the "object_ref(OBJECT(cpu->memory))" in > > cpu_exec_initfn(). > > > > But this case doesn't need to increase ref count like cpu->memory, since > > memory_region_ref() provides protection and it's enough. > > > > This is the difference. > > > > So it sounds like now it's more necessary to clarify this, no? > > > > clarify why smram_as_root doesn't need to be object_ref()'ed explicitly like > what cpu_exec_initfn() does for cpu->memory?
When you're referring cpu->memory, you should aware where's the difference and why you don't need do the same thing. This is necessary for a clear commit message, and it also allows more eyes to check whether it is correct and whether there are any potential problems. Providing details is always beneficial. > As you saide, > > cpu_address_space_init() > -> address_space_init() > -> memory_region_ref() > > it already ensures the ref count is increased. Yes, this is what I mean. > Why cpu_exec_initfn() increases the refcount of cpu->memory, is > totally unrelated to cpu_address_space_init(). ^^^^^^^^^^^^^^^^^ The validity of cpu->memory must be guaranteed, as this is a prerequisite for everything else. And isn't it mainly related with the CPU address space?? It can be said that because the pointer to system_memory needs to be cached in CPUState, such a ref is useful, thereby ensuring the safety of the next address space stuff.