Re: [PATCH v3] vpci: Add resizable bar support

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.12.2024 06:42, Jiqian Chen wrote:
> --- /dev/null
> +++ b/xen/drivers/vpci/rebar.c
> @@ -0,0 +1,130 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2024 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + * Author: Jiqian Chen <Jiqian.Chen@xxxxxxx>
> + */
> +
> +#include <xen/hypercall.h>
> +#include <xen/vpci.h>
> +
> +static void cf_check rebar_ctrl_write(const struct pci_dev *pdev,
> +                                      unsigned int reg,
> +                                      uint32_t val,
> +                                      void *data)
> +{
> +    uint64_t size;
> +    struct vpci_bar *bar = data;
> +
> +    size = PCI_REBAR_CTRL_SIZE(val);
> +    if ( bar->enabled )
> +    {
> +        if ( size == bar->size )
> +            return;
> +
> +        /*
> +        * Refuse to resize a BAR while memory decoding is enabled, as
> +        * otherwise the size of the mapped region in the p2m would become
> +        * stale with the newly set BAR size, and the position of the BAR
> +        * would be reset to undefined.  Note the PCIe specification also
> +        * forbids resizing a BAR with memory decoding enabled.
> +        */
> +        gprintk(XENLOG_ERR,
> +                "%pp: refuse to resize BAR with memory decoding enabled\n",
> +                &pdev->sbdf);
> +        return;
> +    }
> +
> +    if ( !((size >> PCI_REBAR_SIZE_BIAS) &
> +           MASK_EXTR(pci_conf_read32(pdev->sbdf,
> +                                     reg - PCI_REBAR_CTRL + PCI_REBAR_CAP),
> +                                     PCI_REBAR_CAP_SIZES)) )
> +        gprintk(XENLOG_WARNING,
> +                "%pp: new size %#lx is not supported by hardware\n",
> +                &pdev->sbdf, size);

This only covers the 1Mb ... 128Tb range. What about the 256Tb ... 8Eb one?

> +static int cf_check init_rebar(struct pci_dev *pdev)
> +{
> +    uint32_t ctrl;
> +    unsigned int rebar_offset, nbars;
> +
> +    rebar_offset = pci_find_ext_capability(pdev->sbdf, PCI_EXT_CAP_ID_REBAR);
> +
> +    if ( !rebar_offset )
> +        return 0;
> +
> +    if ( !is_hardware_domain(pdev->domain) )
> +    {
> +        printk("ReBar is not supported for domUs\n");
> +        return -EOPNOTSUPP;
> +    }
> +
> +    ctrl = pci_conf_read32(pdev->sbdf, rebar_offset + PCI_REBAR_CTRL);
> +    nbars = MASK_EXTR(ctrl, PCI_REBAR_CTRL_NBAR_MASK);
> +
> +    for ( unsigned int i = 0; i < nbars; i++, rebar_offset += PCI_REBAR_CTRL 
> )

PCI_REBAR_CTRL is an offset; it can't be used to bump rebar_offset here.
That'll need a separate constant, even if both evaluate to 8.

Jan