Hi Luca,
On 10/04/2024 16:08, Luca Fancellu wrote:
>
>
> Hi Michal,
>
>>>
>>> For direct-map domain with iommu on, after we get guest shm info from
>>> "kinfo",
>>> we use "remove_shm_from_rangeset" to remove static shm.
>>>
>>> For direct-map domain with iommu off, as static shm has already been taken
>>> care of through reserved memory banks, we do nothing.
>> Stale info given that shmem is no longer part of reserved memory banks. It's
>> been taken care
>> of by removing shmem regions in find_unallocated_memory()
>
> Sure, will amend for this:
>
>>>
>>> +int __init remove_shm_from_rangeset(const struct kernel_info *kinfo,
>>> + struct rangeset *rangeset)
>>> +{
>>> + const struct membanks *shm_mem = &kinfo->shm_mem.common;
>>> + unsigned int i;
>>> +
>>> + /* Remove static shared memory regions */
>>> + for ( i = 0; i < shm_mem->nr_banks; i++ )
>>> + {
>>> + paddr_t start, end;
>>> + int res;
>>> +
>>> + start = shm_mem->bank[i].start;
>>> + end = shm_mem->bank[i].start + shm_mem->bank[i].size - 1;
>> If you look at other rangeset_remove_range use cases and error messages, 1
>> is subtracted
>> in PFN_DOWN() so that the error message contains end unchanged. Please
>> adhere to that so that
>> printed messages are consistent.
>
> Yes I will change it to have -1 inside PFN_DOWN(), here and in the other
> occurrences
>>>
>>> + /* Remove static shared memory regions */
>>> + res = remove_shm_from_rangeset(kinfo, guest_holes);
>>> + if ( res )
>>> + goto out;
>>> +
>> Could you please add a comment explaining here what's done below?
>
> Is it ok something like this:
I'm ok with your proposal in the other e-mail.
>
> /*
> * Take the interval of memory starting from the first extended region bank
> * start address and ending to the end of the last extended region bank.
I would stop here. The rest reads quite difficult.
> * The interval will be passed to rangeset_report_ranges to allow it to
> * create, by the add_ext_regions callback, a set of extended memory region
> * banks from the guest_holes rangeset, which contains the original extended
> * memory region ranges where the static shared memory ranges are carved
> * out.
> */
>
>>> + i = ext_regions->nr_banks - 1;
>>> + start = ext_regions->bank[0].start;
>>> + end = ext_regions->bank[i].start + ext_regions->bank[i].size - 1;
>>> +
>>> + /* Reset original extended regions to hold new value */
>>> + ext_regions->nr_banks = 0;
>>> + res = rangeset_report_ranges(guest_holes, PFN_DOWN(start),
>>> PFN_DOWN(end),
>>> + add_ext_regions, ext_regions);
>>> + if ( res )
>>> + ext_regions->nr_banks = 0;
>>> + else if ( !ext_regions->nr_banks )
>>> + res = -ENOENT;
>>> +
>>> + out:
>>> + rangeset_destroy(guest_holes);
>>> +
>>> + return res;
>>> +}
>>> +
>>> /*
>>> * Local variables:
>>> * mode: C
>>> --
>>> 2.34.1
>>>
>>
>> ~Michal
>>
>
> Cheers,
> Luca
~Michal
