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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 22/05/2025 3:03 pm, 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

Minor grammar point.  "prevent dom0 from changing".

> 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

"0, as they are not"

> +     * be required by Linux, that can reposition BARs with memory decoding

"Linux, which may reposition".

Otherwise, Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

> +     * enabled.  By returning false here bar->enabled will be set to false, 
> and
> +     * bar_write() will work as expected.
> +     */
> +    if ( mfn_eq(start, _mfn(0)) )
> +        return false;
> +
>      /*
>       * Check if BAR is not overlapping with any memory region defined
>       * in the memory map.