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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.04.2026 10:50, Roger Pau Monné wrote:
> On Mon, Apr 06, 2026 at 03:11:58PM -0400, 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);
> 
> I don't think it matters a lot, but here we are changing the position
> of the BAR in the host memory map while the mappings are still active.

It would matter if the original address space could be re-used for another
purpose while those mappings are still there?

Jan