Re: [PATCH 7/9] vpci/header: program p2m with guest BAR view

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.09.2021 11:16, Oleksandr Andrushchenko wrote:
> 
> On 09.09.21 11:26, Jan Beulich wrote:
>> On 09.09.2021 08:13, Oleksandr Andrushchenko wrote:
>>> On 06.09.21 17:51, Jan Beulich wrote:
>>>> On 03.09.2021 12:08, Oleksandr Andrushchenko wrote:
>>>>> @@ -37,12 +41,28 @@ static int map_range(unsigned long s, unsigned long 
>>>>> e, void *data,
>>>>>                         unsigned long *c)
>>>>>    {
>>>>>        const struct map_data *map = data;
>>>>> +    gfn_t start_gfn;
>>>>>        int rc;
>>>>>    
>>>>>        for ( ; ; )
>>>>>        {
>>>>>            unsigned long size = e - s + 1;
>>>>>    
>>>>> +        /*
>>>>> +         * Any BAR may have holes in its memory we want to map, e.g.
>>>>> +         * we don't want to map MSI regions which may be a part of that 
>>>>> BAR,
>>>>> +         * e.g. when a single BAR is used for both MMIO and MSI.
>>>>> +         * In this case MSI regions are subtracted from the mapping, but
>>>>> +         * map->start_gfn still points to the very beginning of the BAR.
>>>>> +         * So if there is a hole present then we need to adjust start_gfn
>>>>> +         * to reflect the fact of that substraction.
>>>>> +         */
>>>>> +        start_gfn = gfn_add(map->start_gfn, s - mfn_x(map->start_mfn));
>>>> I may be missing something, but don't you need to adjust "size" then
>>>> as well?
>>> No, as range sets get consumed we have e and s updated accordingly,
>>> so each time size represents the right value.
>> It feels like something's wrong with the rangeset construction then:
>> Either it represents _all_ holes (including degenerate ones at the
>> start of end of a range), or none of them.
> 
> The resulting range set only has the MMIOs in it. While constructing the 
> range set
> we cut off MSI-X out of it (make holes). But finally it only has the ranges 
> that we
> need to map/unmap.

And then why is there a need to adjust start_gfn?

Jan