Re: [PATCH PROTOTYPE 1/6] memory: Introduce sparse RAM handler for memory regions

[David Hildenbrand]

On 20.10.20 21:24, Peter Xu wrote:
> On Thu, Sep 24, 2020 at 06:04:18PM +0200, David Hildenbrand wrote:
>> +static inline void memory_region_set_sparse_ram_handler(MemoryRegion *mr,
>> +                                                        SparseRAMHandler 
>> *srh)
>> +{
>> +    g_assert(memory_region_is_ram(mr));
> 
> Nit: Maybe assert mr->srh==NULL here?  If sparse ram handler is exclusive,
> which afaiu, is a yes.

Indeed. The owner of the memory region.

> 
>> +    mr->srh = srh;
>> +}
>> +
>> +static inline void memory_region_register_sparse_ram_notifier(MemoryRegion 
>> *mr,
>> +                                                           
>> SparseRAMNotifier *n)
>> +{
>> +    SparseRAMHandler *srh = memory_region_get_sparse_ram_handler(mr);
>> +    SparseRAMHandlerClass *srhc = SPARSE_RAM_HANDLER_GET_CLASS(srh);
>> +
>> +    srhc->register_listener(srh, mr, n);
> 
> I feel like you need to check srhc!=NULL first or vfio may start crash without
> virtio-mem...  Same question to the other ones (at least unregister).

I think nobody should be calling this function unless
memory_region_is_sparse_ram() returns true.

I'm still considering moving it down to RAMBlock level. Feels more
natural, and would allow other RAMBlock users figure out what's going
on. If only my list of TODO items wouldn't be that long ... :)

Thanks Peter!

-- 
Thanks,

David / dhildenb