Hi Oleksandr,
On 23.09.2021 14:54, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com>
>
> 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_andrushche...@epam.com>
>
> ---
> Since v1:
> - s/MSI/MSI-X in comments
> ---
> xen/drivers/vpci/header.c | 35 +++++++++++++++++++++++++++++++++--
> 1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index 9c603d26d302..bdd18599b205 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -30,6 +30,10 @@
>
> struct map_data {
> struct domain *d;
> + /* Start address of the BAR as seen by the guest. */
> + gfn_t start_gfn;
> + /* Physical start address of the BAR. */
> + mfn_t start_mfn;
> bool map;
> };
>
> @@ -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.
> + * 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
> + "%smap [%lx, %lx] -> %#"PRI_gfn" for d%d\n",
> + map->map ? "" : "un", s, e, gfn_x(start_gfn),
> + map->d->domain_id);
> /*
> * ARM TODOs:
> * - On ARM whether the memory is prefetchable or not should be
> passed
> @@ -52,8 +72,10 @@ static int map_range(unsigned long s, unsigned long e,
> void *data,
> * - {un}map_mmio_regions doesn't support preemption.
> */
>
> - rc = map->map ? map_mmio_regions(map->d, _gfn(s), size, _mfn(s))
> - : unmap_mmio_regions(map->d, _gfn(s), size, _mfn(s));
> + rc = map->map ? map_mmio_regions(map->d, start_gfn,
> + size, _mfn(s))
> + : unmap_mmio_regions(map->d, start_gfn,
> + size, _mfn(s));
> if ( rc == 0 )
> {
> *c += size;
> @@ -69,6 +91,7 @@ static int map_range(unsigned long s, unsigned long e, void
> *data,
> ASSERT(rc < size);
> *c += rc;
> s += rc;
> + gfn_add(map->start_gfn, rc);
> if ( general_preempt_check() )
> return -ERESTART;
> }
> @@ -149,6 +172,10 @@ bool vpci_process_pending(struct vcpu *v)
> if ( !bar->mem )
> continue;
>
> + data.start_gfn = is_hardware_domain(v->vpci.pdev->domain) ?
> + _gfn(PFN_DOWN(bar->addr)) :
> + _gfn(PFN_DOWN(bar->guest_addr));
I believe this would look better with the following alignment:
data.start_gfn = is_hardware_domain(v->vpci.pdev->domain)
? _gfn(PFN_DOWN(bar->addr))
: _gfn(PFN_DOWN(bar->guest_addr));
> + data.start_mfn = _mfn(PFN_DOWN(bar->addr));
> rc = rangeset_consume_ranges(bar->mem, map_range, &data);
>
> if ( rc == -ERESTART )
> @@ -194,6 +221,10 @@ static int __init apply_map(struct domain *d, const
> struct pci_dev *pdev,
> if ( !bar->mem )
> continue;
>
> + data.start_gfn = is_hardware_domain(d) ?
> + _gfn(PFN_DOWN(bar->addr)) :
> + _gfn(PFN_DOWN(bar->guest_addr));
This one also.
> + data.start_mfn = _mfn(PFN_DOWN(bar->addr));
> while ( (rc = rangeset_consume_ranges(bar->mem, map_range,
> &data)) == -ERESTART )
> process_pending_softirqs();
>
Cheers,
Michal
