Re: [PATCH 2/2] x86/vpci: refuse to map BARs at position 0

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

On 5/22/25 10:03, Roger Pau Monne wrote:
> A BAR at position 0 is not initialized (not positioned).  While Xen could
> attempt to map it into the p2m, marking it as mapped will prevent dom0 to
> change the position of the BAR, as the vPCI code has a shortcomming of not
> allowing to write to BAR registers while the BAR is mapped on the p2m.
> 
> Workaround this limitation by returning false from pci_check_bar() if the
> BAR address is 0, thus causing the bar->enabled field to also be set to
> false and allowing bar_write() to change the BAR position.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
>  xen/arch/x86/pci.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/xen/arch/x86/pci.c b/xen/arch/x86/pci.c
> index 26bb7f6a3c3a..39fd5a16a4aa 100644
> --- a/xen/arch/x86/pci.c
> +++ b/xen/arch/x86/pci.c
> @@ -101,6 +101,15 @@ int pci_conf_write_intercept(unsigned int seg, unsigned 
> int bdf,
>  
>  bool pci_check_bar(const struct pci_dev *pdev, mfn_t start, mfn_t end)
>  {
> +    /*
> +     * Refuse to map BARs at position 0, those are not initialized.  This 
> might
> +     * be required by Linux, that can reposition BARs with memory decoding
> +     * enabled.  By returning false here bar->enabled will be set to false, 
> and
> +     * bar_write() will work as expected.
> +     */

Technically speaking, the particular corner case is plausible.

However, if I understand it correctly, when Linux finds an uninitialized
BAR, it checks if the BAR (resource) has been allocated, and won't
enable memory decoding if unallocated. See Linux
drivers/pci/setup-res.c:pci_enable_resources().

So I would consider dropping the "This might be required by Linux"
part from the comment.

> +    if ( mfn_eq(start, _mfn(0)) )
> +        return false;
> +
>      /*
>       * Check if BAR is not overlapping with any memory region defined
>       * in the memory map.