Re: [PATCH v5 06/13] hw/mem/cxl_type3: Add host backend and address space handling for DC regions

[Jonathan Cameron]

> > > @@ -868,16 +974,24 @@ static int cxl_type3_hpa_to_as_and_dpa(CXLType3Dev 
> > > *ct3d,
> > >                                         AddressSpace **as,
> > >                                         uint64_t *dpa_offset)
> > >  {
> > > -    MemoryRegion *vmr = NULL, *pmr = NULL;
> > > +    MemoryRegion *vmr = NULL, *pmr = NULL, *dc_mr = NULL;
> > > +    uint64_t vmr_size = 0, pmr_size = 0, dc_size = 0;
> > >  
> > >      if (ct3d->hostvmem) {
> > >          vmr = host_memory_backend_get_memory(ct3d->hostvmem);
> > > +        vmr_size = memory_region_size(vmr);
> > >      }
> > >      if (ct3d->hostpmem) {
> > >          pmr = host_memory_backend_get_memory(ct3d->hostpmem);
> > > +        pmr_size = memory_region_size(pmr);
> > > +    }
> > > +    if (ct3d->dc.host_dc) {
> > > +        dc_mr = host_memory_backend_get_memory(ct3d->dc.host_dc);
> > > +        /* Do we want dc_size to be dc_mr->size or not?? */  
> > 
> > Maybe - definitely don't want to leave this comment here
> > unanswered and I think you enforce it above anyway.
> > 
> > So if we get here ct3d->dc.total_capacity == 
> > memory_region_size(ct3d->dc.host_dc);
> > As such we could just not stash total_capacity at all?  
> 
> I cannot identify a case where these two will be different. But
> total_capacity is referenced at quite some places, it may be nice to have
> it so we do not need to call the function to get the value every time?

I kind of like having it via one path so that there is no confusion
for the reader, but up to you on this one.  The function called is trivial
(other than some magic to handle very large memory regions) so
this is just a readability question, not a perf one.

Whatever, don't leave the question behind.  Find to have something
that says they are always the same size if you don't get rid
of the total_capacity representation.


Jonathan