On Thu, Mar 07, 2024 at 12:16:05PM +0000, Jonathan Cameron wrote:
> > > > @@ -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.
>
I will fix it.
For static capability, we have a variable static_mem_size, although we
can calculate it from volatile and non-volatile memory region size.
There are quite some places need to get the dynamic capacity, it is much
more convenient to have a variable ready to use, I will keep it for
now.
Fan
>
> Jonathan
