Re: [PATCH v3 07/11] vpci/header: program p2m with guest BAR view

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.09.2021 09:52, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> 
> Take into account guest's BAR view and program its p2m accordingly:
> gfn is guest's view of the BAR and mfn is the physical BAR value as set
> up by the host bridge in the hardware domain.
> This way hardware doamin sees physical BAR values and guest sees
> emulated ones.
> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>

Just a couple of nits, as I remain unconvinced of the rangeset related
choice in the earlier patch.

> @@ -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-X regions which may be a part of that 
> BAR,
> +         * e.g. when a single BAR is used for both MMIO and MSI-X.

This second "e.g." seems, to me at least, quite redundant with the first
one.

> +         * In this case MSI-X 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));
> +
> +        printk(XENLOG_G_DEBUG

Do you really mean this to be active even in release builds? Might get
quite noisy ...

> +               "%smap [%lx, %lx] -> %#"PRI_gfn" for d%d\n",

%pd please in new or altered code.

Jan