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

["Alejandro Vallejo" <alejandro.vallejo@xxxxxxxxx>]

On Thu Mar 13, 2025 at 5:43 PM GMT, Stewart Hildebrand wrote:
> The idea was that the unmap-then-map operation would appear atomic from
> the guest's point of view. I've only queued up the unmap operation at
> this point in the new logic. Due to the mentioned limitation in the BAR
> mapping deferral machinery, I wanted to make sure *this BAR* was
> unmapped before queuing up the map operation (see below). Waiting for
> *all* pending operations to finish here is likely not appropriate.

Looking more closely after reading Roger's answer, I misunderstood what was
being queued where. There's space for a single deferred operation that's
retried if pending on each attempt to resume the vCPU, whereas I initially
thought it was the mutations to the p2m (which would've competed with other
mutations from other vCPUs). This makes more sense, sorry for the noise.

> I think this just reinforces the need to rework the BAR mapping
> machinery.

Right. The most delicate part is dealing with races with another vCPU when the
unmap-then-map operation does not complete in a single taking of the vpci lock
I'd say. And that much is unavoidable, I think, because either unmapping or
mapping might take a while.

>
> > Do you know if Linux intentionally skips disabling decode? Or is it a bug?
>
> I think it's intentional. See https://gitlab.com/xen-project/xen/-/issues/197

Interesting. I seemed to recall some devices being able to decode their own BAR
accesses. But I must've been wrong.

>
> >> +            }
> >> +        }
> >> +        else
> >> +            return;
> >>      }
> >>  
> >>  
> >> @@ -610,6 +647,10 @@ static void cf_check bar_write(
> >>      }
> >>  
> >>      pci_conf_write32(pdev->sbdf, reg, val);
> >> +
> >> +    if ( reenable )
> >> +        /* Write-while-mapped: map the new BAR in p2m. OK to defer. */
> >> +        modify_bars(pdev, cmd, false);
>
> This call to modify_bars() will raise a softirq for the map operation.

Ah, fair enough. I clearly didn't look closely enough.

Cheers,
Alejandro