Re: [PATCH v4 4/4] vpci: allow 32-bit BAR writes with memory decoding enabled

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.04.2026 21:11, Stewart Hildebrand wrote:
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -670,6 +670,7 @@ static void cf_check bar_write(
>  {
>      struct vpci_bar *bar = data;
>      bool hi = false;
> +    uint16_t cmd = 0;
>  
>      ASSERT(is_hardware_domain(pdev->domain));
>  
> @@ -683,19 +684,29 @@ static void cf_check bar_write(
>          val &= PCI_BASE_ADDRESS_MEM_MASK;
>  
>      /*
> -     * Xen only cares whether the BAR is mapped into the p2m, so allow BAR
> -     * writes as long as the BAR is not mapped into the p2m.
> +     * Allow 64-bit BAR writes only when the BAR is not mapped in p2m. Always
> +     * allow 32-bit BAR writes.
>       */
>      if ( bar->enabled )
>      {
> -        /* If the value written is the current one avoid printing a warning. 
> */
> -        if ( val != (uint32_t)(bar->addr >> (hi ? 32 : 0)) )
> -            gprintk(XENLOG_WARNING,
> -                    "%pp: ignored BAR %zu write while mapped\n",
> -                    &pdev->sbdf, bar - pdev->vpci->header.bars + hi);
> -        return;
> -    }
> +        if ( bar->type == VPCI_BAR_MEM32 )
> +        {
> +            if ( val == bar->addr )
> +                return;
>  
> +            cmd = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
> +            modify_bars(pdev, cmd, false, false);
> +        }
> +        else
> +        {
> +            /* If the value written is the same avoid printing a warning. */
> +            if ( val != (uint32_t)(bar->addr >> (hi ? 32 : 0)) )
> +                gprintk(XENLOG_WARNING,
> +                        "%pp: ignored BAR %zu write while mapped\n",
> +                        &pdev->sbdf, bar - pdev->vpci->header.bars + hi);
> +            return;
> +        }
> +    }
>  
>      /*
>       * Update the cached address, so that when memory decoding is enabled
> @@ -715,6 +726,9 @@ static void cf_check bar_write(
>      }
>  
>      pci_conf_write32(pdev->sbdf, reg, val);
> +
> +    if ( bar->enabled )
> +        modify_bars(pdev, cmd, false, true);
>  }
>  
>  static void cf_check guest_mem_bar_write(const struct pci_dev *pdev,

While this all looks plausible, isn't something similar needed in rom_write()
then as well?

Jan