Re: [PATCH v4 3/4] vpci: allow BAR map/unmap without affecting memory decoding bit

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Apr 06, 2026 at 03:11:57PM -0400, Stewart Hildebrand wrote:
> Introduce 'bool map' and allow invoking modify_bars() without changing
> the memory decoding bit. This will allow hardware domain to reposition
> BARs without affecting the memory decoding bit.
> 
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
> ---
> This also lays some groundwork to allow domUs to toggle the guest view
> without affecting hardware, a step toward addressing the FIXME in [1].
> 
> [1] 
> https://lore.kernel.org/xen-devel/20250814160358.95543-4-roger.pau@xxxxxxxxxx/

The deferred toggling of the command register bits seems overly
complex IMO.  It would be much simpler if domUs simply don't get to
play with the real decoding bits.  For the hardware domain we could
toggle the decoding bits ahead of the mapping operations, so that we
don't have to defer the toggling after mappings have been sorted.

Long term we should aim to remove the cmd field from vpci_map_task
struct.

I don't mind if you want to pick the patch in [1] in your series, as
we could then drop the cmd field and possibly simplify some of the
logic in this patch by no longer requiring the deferred write to the
command register?

> v3->v4:
> * rebase on dynamically allocated map queue
> 
> v2->v3:
> * use bool
> * switch to task->map in more places
> 
> v1->v2:
> * new patch
> ---
>  xen/drivers/vpci/header.c | 43 +++++++++++++++++++++------------------
>  xen/include/xen/vpci.h    |  1 +
>  2 files changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index 146915e28c50..20fe380552f4 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -117,11 +117,12 @@ static int cf_check map_range(
>   * BAR's enable bit has changed with the memory decoding bit already enabled.
>   * If rom_only is not set then it's the memory decoding bit that changed.
>   */
> -static void modify_decoding(const struct pci_dev *pdev, uint16_t cmd,
> -                            bool rom_only)
> +static void modify_decoding(const struct pci_dev *pdev,
> +                            struct vpci_map_task *task)

task wants to be const here, unless further patches require modifying
task inside of modify_decoding().

Thanks, Roger.