Re: [PATCH] [RFC] vpci: allow BAR write while mapped

["Roger Pau Monné" <roger.pau@citrix.com>]

On Fri, Mar 14, 2025 at 09:04:55AM +0100, Jan Beulich wrote:
> On 13.03.2025 18:48, Roger Pau Monné wrote:
> > On Wed, Mar 12, 2025 at 03:50:17PM -0400, Stewart Hildebrand wrote:
> >> @@ -585,10 +601,31 @@ static void cf_check bar_write(
> >>      {
> >>          /* 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",
> >> +                    "%pp: allowing BAR %zu write while mapped\n",
> >>                      &pdev->sbdf, bar - pdev->vpci->header.bars + hi);
> > 
> > If Xen now handles BARs writes with memory decoding enabled the
> > message can be removed.  It's only purpose was to signal this missing
> > handling.
> > 
> >> -        return;
> >> +            ASSERT(rw_is_write_locked(&pdev->domain->pci_lock));
> >> +            ASSERT(spin_is_locked(&pdev->vpci->lock));
> >> +            reenable = true;
> >> +            cmd = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
> >> +            /*
> >> +             * Write-while-mapped: unmap the old BAR in p2m. We want this to
> >> +             * finish right away since the deferral machinery only supports
> >> +             * unmap OR map, not unmap-then-remap. Ultimately, it probably would
> >> +             * be better to defer the write-while-mapped case just like regular
> >> +             * BAR writes (but still only allow it for 32-bit BAR writes).
> >> +             */
> >> +            /* Disable memory decoding */
> >> +            modify_bars(pdev, cmd & ~PCI_COMMAND_MEMORY, false);
> > 
> > I think if the guest explicitly avoids toggling memory decoding Xen
> > should also to the same, and not touch the bit.
> 
> For Dom0 I'm inclined to agree, but for DomU-s it may be unsafe to do so.
> (You may have meant it like this, but you said "guest".)

Sorry, I'm not sure I'm following.  For domUs the BAR register write
is not propagated to the hardware, it's just the p2m mappings that
change, and hence it's even safer to not toggle the memory decoding
bit for that case? (as there's no write to the device BAR registers
for domUs).

Maybe there's some aspect I'm missing.

Thanks, Roger.